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

Unexpected Behavior with Monkey Patch from PR #54752 causing Double Encoding of Dynamic Segments #57829

Closed
1 task done
jianliao opened this issue Oct 31, 2023 · 2 comments · Fixed by #57960
Closed
1 task done
Assignees
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked

Comments

@jianliao
Copy link

jianliao commented Oct 31, 2023

Link to the code that reproduces this issue

https://github.com/jianliao/nextjs-webpack-double-encoding

To Reproduce

  1. npm install
  2. npm run dev
  3. Click the button Test Routing

Current vs. Expected behavior

The application crashed and below error pops up:

Unhandled Runtime Error
ChunkLoadError: Loading chunk app/blog/[slug]/error failed.
(error: https://z57v4c-3000.csb.app/_next/static/chunks/app/blog/%255Bslug%255D/error.js)
...

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.1.0: Mon Oct  9 21:27:24 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T6000
Binaries:
  Node: 20.8.0
  npm: 10.1.0
  Yarn: N/A
  pnpm: N/A
Relevant Packages:
  next: 14.0.2-canary.1
  eslint-config-next: 14.0.1
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.2.2
Next.js Config:
  output: export

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

App Router

Additional context

Here is a quick codesandbox for reproduction.
In the recent merge of Pull Request #54752, a monkey patch was introduced to the Next.js embedded webpack configuration. This patch is found to behave unexpectedly when a beforeInteractive script is part of the project. Particularly, the code within app-webpack.ts at lines 18 and 38 is triggered, which in turn causes a double encoding issue with dynamic segments such as [slug]. This double encoding prevents the development server from correctly loading the JavaScript file associated with the dynamic segment.

NEXT-1707

@jianliao jianliao added the bug Issue was opened via the bug report template. label Oct 31, 2023
@jianliao
Copy link
Author

Looks like this issue duplicated with #56484.

@huozhi huozhi self-assigned this Nov 2, 2023
@huozhi huozhi added the linear: next Confirmed issue that is tracked by the Next.js team. label Nov 2, 2023
@kodiakhq kodiakhq bot closed this as completed in #57960 Nov 2, 2023
kodiakhq bot pushed a commit that referenced this issue Nov 2, 2023
We had added encoding the client component assets loaded from RSC manifest that we need to encode them to make sure when they're loaded on server and sent to client, the client will receive the encoded one. But the override of the webpack chunk loading method could be loaded later than react related chunks, that when client component is loaded first (e.g. `next/script`) and it triggers react loaded ealier than the overriding. Then the chunk could be encoded incorrectly.

Discussed with @gnoff and put this out as the 1st step solution to ensure the order. in the future we can try to get rid of the encoding by providing safer url

Fixes #57829
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 Nov 17, 2023
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

Successfully merging a pull request may close this issue.

2 participants