Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrites not working in afterFiles when they start with assetPrefix #64710

Closed
dlehmhus opened this issue Apr 18, 2024 · 6 comments
Closed

Rewrites not working in afterFiles when they start with assetPrefix #64710

dlehmhus opened this issue Apr 18, 2024 · 6 comments
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked

Comments

@dlehmhus
Copy link
Contributor

dlehmhus commented Apr 18, 2024

Link to the code that reproduces this issue

https://github.com/dlehmhus/next-rewrite-issue

https://stackblitz.com/github/dlehmhus/next-rewrite-issue?file=next.config.js,app%2Fapi%2Frewrite-api%2Froute.ts

To Reproduce

  1. Start the application
  2. Go to "/asset-prefix/api/rewrite-api"
  3. Compare to "/not-asset-prefix/api/rewrite-api"

Current vs. Expected behavior

We expect to see in step 2. the same result as in step 3., both paths should be rewritten to "/api/rewrite-api". But since Next.js 14.2 when the rewrite path is similar to the asset prefix, the request will not get rewritten at all.

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.2.0: Wed Nov 15 21:53:18 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T6000
  Available memory (MB): 16384
  Available CPU cores: 10
Binaries:
  Node: 20.10.0
  npm: 10.2.3
  Yarn: 1.22.19
  pnpm: 8.14.0
Relevant Packages:
  next: 14.3.0-canary.9 // Latest available version is detected (14.3.0-canary.9).
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.1.3
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Not sure

Which stage(s) are affected? (Select all that apply)

next dev (local), next start (local)

Additional context

The issue was introduced in v14.2.0-canary.50, my guess is this MR !63627

NEXT-3172

@dlehmhus dlehmhus added the bug Issue was opened via the bug report template. label Apr 18, 2024
@wyattjoh wyattjoh added the linear: next Confirmed issue that is tracked by the Next.js team. label Apr 19, 2024
@dannyrb
Copy link

dannyrb commented Apr 19, 2024

I shared this with our Vercel CSM, but this is impacting us as well. Our use case:

We use assetPrefix as a way to accomplish Vercel's "Multi Zones" pattern. The alternative is to use basePath, but that introduces additional limitations in middleware and with the number of path segments a Vercel app can handle requests for.

To illustrate the point, we have 10+ Vercel apps that handle requests for the same domain:

  • App One
    • *
  • App Two (not possible w/ basePath)
    • /some-path*
    • /another-path*
  • App Etc.
    • /some-other-path*

We have Cloudflare worker that handles all requests for the domain www.example.com. If the path for the request matches a route configured for an expression owned by an app, we do something like a "Bulk Origin Override" so the target app can handle that request. Any requests without a match pass through and hit App One.

Why is this an issue?

All apps are looking for their _next/ assets at the same path. It's impossible to route them when the path prefix is not unique. Each app has to specify an assetPrefix that is unique, and then rewrites needs to be used so the Next.js/Vercel app can return them for that unique path.

basePath Limitations

  • We need some applications to have what amounts to more than one basePath
  • It changes the middleware match functionality
    • This breaks how we use/test preview environments (but I'll avoid getting into that)
  • We have less control over how non basePath* routes are handled

@Netail
Copy link
Contributor

Netail commented May 11, 2024

@dlehmhus Are you sure you have to use afterFiles? The docs mention this occurs after fetching pages.
beforeFiles instead of afterFiles does seem to work in your example usage. it might have worked before as it didn't remove the asset-prefix from the internal path

@dannyrb For our micro frontend usage we set an assetPrefix per app (path corresponding to the app name) so it doesn't conflict in the _next bundle fetching. Previously the bundles would return a 404 with a relative assetPrefix, as it was not removed from the internal path, which was solved with a simple rewrite workaround to the path without an assetPrefix

@dlehmhus
Copy link
Contributor Author

@Netail as the docs state the afterFiles rewrites happen after Filesystems routes. That is unfortunately exactly what I need in my case 😞

@mknichel
Copy link
Member

#68694 should fix this issue.

ijjk pushed a commit that referenced this issue Aug 15, 2024
…etPrefix (#68694)

This PR fixes two issues with the use of `assetPrefix`:

#1: #64710
`assetPrefix` needs to be handled in `dev`, `deploy`, and `start`. In
the current approach, only `dev` and `start` were handled, but a quirk
of the implementation caused rewrites for non-asset paths to not be able
to be used in `afterFiles` rewrites.

#2: When deploying Next.js (such as on Vercel), you need to add your own
`beforeFiles` rewrite for `/${assetPrefix}/_next/...` requests or
otherwise they would 404.

This PR creates an automatically added `rewrite` to `beforeFiles` that
handles the case for `dev`, `start`, and `deploy`, removes the existing
logic in `filesystem.ts`, and adds more tests to check the behavior.
@ijjk
Copy link
Member

ijjk commented Oct 2, 2024

Closing per-above as this should be resolved now

@ijjk ijjk closed this as completed Oct 2, 2024
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked
Projects
None yet
Development

No branches or pull requests

6 participants