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

[NEXT-659] Next.js 13 is broken for custom server with multiple zones #45852

Closed
1 task done
sirrodgepodge opened this issue Feb 13, 2023 · 18 comments · Fixed by #46533
Closed
1 task done

[NEXT-659] Next.js 13 is broken for custom server with multiple zones #45852

sirrodgepodge opened this issue Feb 13, 2023 · 18 comments · Fixed by #46533
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team.

Comments

@sirrodgepodge
Copy link

sirrodgepodge commented Feb 13, 2023

Verify canary release

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

Provide environment information

Node 16.19.0
Yarn 3.1.4

Previously all worked on Next version 12.1.5

Which area(s) of Next.js are affected? (leave empty if unsure)

No response

Link to the code that reproduces this issue

https://github.com/sirrodgepodge/busted-zones

To Reproduce

Following the instructions in the README, start by running

yarn && yarn dev

Mystery #1

First visit

http://localhost:3000/guest

See that ./hotel-guest/pages/_app.tsx gets rendered ("Guest App" and the top left), but the component renders a 404, even though ./hotel-guest/pages/index.tsx is defined.

Mystery #2

Even more perplexing... now visit

http://localhost:3000/host

See that ./hotel-host/pages/_app.tsx gets rendered ("Host App" and the top left), but the component, instead of ./hotel-host/pages/index.tsx renders the content of ./hotel-guest/pages/index.tsx!!!

For mystery #1, I have no idea, for mystery #2, I can only conclude that there's a global next.js pages cache of some sort (independent of next.js app instances).

Describe the Bug

I've upgraded to Next 13 and I'm seeing some insane behavior.

First there's an issue with just rendering standard pages where it's a 404.

Second, is as if there is a global next.js cache that is being referenced across multiple next app instances (I.e. "zones") because when visiting the second app instance, the second _app.tsx is rendered and the first index.tsx is rendered.

Expected Behavior

These apps should render independently.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

NEXT-659

@sirrodgepodge sirrodgepodge added the bug Issue was opened via the bug report template. label Feb 13, 2023
@sirrodgepodge
Copy link
Author

@daiyam
Copy link

daiyam commented Feb 14, 2023

@sirrodgepodge Based on the doc https://nextjs.org/docs/advanced-features/multi-zones#how-to-merge-zones and the example https://github.com/vercel/next.js/tree/canary/examples/with-zones

For development, you need to run each zones separately.

For deployement, you have to configure Vercel or NGinx to redirect the request to there respective zones. (https://blog.logrocket.com/exploring-zones-in-next-js/)

@sirrodgepodge
Copy link
Author

sirrodgepodge commented Feb 14, 2023

As in they need to be run in separate processes?

Why is this? Intuitively I don't see why the code I have wouldn't work (it used to lol).

I get that that's how the example is done but what I'm doing seems like it would be a perfectly valid alternative

@daiyam
Copy link

daiyam commented Feb 14, 2023

I haven't seen that the backend was in fact a reverse proxy with Express...

When testing your example with Next v12, _app.tsx is rendered but index.tsx also fails but with the error:

error - ../hotel-guest/pages/index.tsx
Module parse failed: Unexpected token (3:4)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| export default function HotelGuest() {
|   return (
>     <div>
|       Guest Index
|     </div>

@sirrodgepodge
Copy link
Author

sirrodgepodge commented Feb 15, 2023

hmm next 12.1.5 it for sure works

I think this is an unrelated error that just has to do with webpack loader config

@sirrodgepodge
Copy link
Author

@morganjng
Copy link

I'm looking into this right now and it is a pretty interesting bug (or feature, if it is an intended side effect). There is certainly some global state that acts weird with multiple NextServers.

Not sure if this is all intended or not, but either way there's definitely some inconsistency in how things operate.

@parvit
Copy link

parvit commented Feb 25, 2023 via email

@parvit
Copy link

parvit commented Feb 26, 2023

Also for the changes required to the demo app: sirrodgepodge/busted-zones#1

@sirrodgepodge
Copy link
Author

holy bejeesus, thank you, looking

@timneutkens timneutkens added the linear: next Confirmed issue that is tracked by the Next.js team. label Feb 27, 2023
@timneutkens timneutkens changed the title Next.js 13 is broken for custom server with multiple zones [NEXT-659] Next.js 13 is broken for custom server with multiple zones Feb 27, 2023
@sirrodgepodge
Copy link
Author

sirrodgepodge commented Feb 27, 2023

thank you @parvit - this looks great, it is still super unintuitive that one must change the app component names tbh, sounds like maybe that's a webpack issue? But ofc it doesn't affect a normal application with two components named the same.

regardless, thank you so much for figuring this one out, as soon as it's merged into a non-canary version of Next.js will release the bounty

@parvit
Copy link

parvit commented Feb 27, 2023 via email

ijjk added a commit that referenced this issue Feb 28, 2023
This ensures we don't keep `entries` and `invalidator` in module scope
directly and nest it under a key specific to each compilation so
multiple `next` instances in the same process don't override one and
another.

## Bug

- [x] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

Closes: #46432
Fixes: #45852
@ijjk
Copy link
Member

ijjk commented Feb 28, 2023

Hi, this should now be patched in v13.2.2-canary.4 of Next.js, please update and give it a try!

@parvit
Copy link

parvit commented Mar 1, 2023

@ijjk I've tested on my end from a fresh checkout and i can confirm that only updating the version to v13.2.2-canary.4 fixes the issues.

@sirrodgepodge
Copy link
Author

@parvit - to be clear, you're saying you don't need to make any of your other changes (e.g. changing the app component names), you just need to update to this version, correct?

It's a decent amount of work to update our actual app to confirm so I just want to be sure.

@parvit
Copy link

parvit commented Mar 4, 2023

@sirrodgepodge Yes i just updated the next version in all the package.json files and it worked.

@sirrodgepodge
Copy link
Author

@parvit thank you!! confirmed - bounty released :)

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. 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 Apr 10, 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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants