withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
46.86k stars 2.49k forks source link

Build failed using `rest params` with static file endpoints. #11537

Closed Trenrod closed 3 months ago

Trenrod commented 3 months ago

Astro Info

Astro                    v4.12.2
Node                     v22.1.0
System                   Linux (x64)
Package Manager          npm
Output                   server
Adapter                  @astrojs/node
Integrations             astro-icon

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Bug description

Unexpected ? at 14, expected END: https://git.new/pathToRegexpError
  Stack trace:
    at Iter.consume (/home/ado/dev/astro/familytree/node_modules/path-to-regexp/dist/index.js:106:9)
    at compile (/home/ado/dev/astro/familytree/node_modules/path-to-regexp/dist/index.js:195:50)
    at deserializeRouteData (file:///home/ado/dev/astro/familytree/dist/server/chunks/_@astrojs-manifest_8j_kPJ8h.mjs:308:15)
    at file:///home/ado/dev/astro/familytree/dist/server/chunks/_@astrojs-manifest_8j_kPJ8h.mjs:351:18
    at async ModuleLoader.import (node:internal/modules/esm/loader:474:24)

Logging path fails while trying to process /api/:path(.*)?. The logic itself works as expected. All /api/... calls terminates in [...path].ts.

My filestructure for /api/ looks like this.

/pages/
-- api/
---- [...path].ts

File [...path].ts looks like this.

import APIRouter from "../../libs/helper/APIRouter";
import type { APIRoute } from "astro";

export const ALL: APIRoute = async (context) => {
    return APIRouter.handleRequest(context);
};

What's the expected result?

Build should be successfull.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-grnxjj?file=src%2Fpages%2Fapi%2F%5B...path%5D.ts

Participation

Trenrod commented 3 months ago

Nailed it down to this line.

// /node_modules/astro/dist/core/routing/manifest/generator.js
function getRouteGenerator(segments, addTrailingSlash) {
    const template = segments.map((segment) => {
        return "/" + segment.map((part) => {
            if (part.spread) {
                return `:${part.content.slice(3)}(.*)?`; <= Removeing this ? allows it to build.

With ?:

  1. /api/:path(.*)? without:
  2. /api/:path(.*)

Seems like pathToRegexpError cannot handle the 1st one. I also dont understand any disadvantages of the 2nd on as I asume there are no grouping used?

Trenrod commented 3 months ago

Checking https://www.npmjs.com/package/path-to-regexp

They mentioning this example

Unnamed parameters

It is possible to define a parameter without a name. The name will be numerically indexed:

const regexp = pathToRegexp("/:foo/(.*)");
// keys = [{ name: 'foo', ... }, { name: '0', ... }]

According to this example the fixed line should look like this? ``return:${part.content.slice(3)}/(.*)````

Trenrod commented 3 months ago

Any adjustments above make it fail in

// node_modules/astro/node_modules/path-to-regexp/dist/index.js:tokensToFunction
            throw new TypeError("Expected \"".concat(token.name, "\" to be ").concat(typeOfMessage));

So to support it I would need to fix it here also?

I would look into it but I would need some guidance if im on the right track.

github-actions[bot] commented 3 months ago

Hello @Trenrod. Please provide a minimal reproduction using a GitHub repository or StackBlitz. Issues marked with needs repro will be closed if they have no activity within 3 days.

ematipico commented 3 months ago

Can you please provide a working reproduction? It doesn't build, and the dev server works as expected

Trenrod commented 3 months ago

After trying to create a new project with a minimal reproduction I found out that the issue was that I was using an own path-to-regexp. Which caused issues with the one supported by Astro.

/// package.json:dependencies
        "path-to-regexp": "^7.0.0",

After removing it build + worked again.