-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix(nextjs): Fix HMR by inserting new entrypoints at the end #9267
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So many edge cases... 🥲
newEntryPoint = isDevMode ? [currentEntryPoint, ...filesToInsert] : [...filesToInsert, currentEntryPoint]; | ||
} else if (Array.isArray(currentEntryPoint)) { | ||
newEntryPoint = [...filepaths, ...currentEntryPoint]; | ||
newEntryPoint = isDevMode ? [...currentEntryPoint, ...filesToInsert] : [...filesToInsert, ...currentEntryPoint]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: took me a while to understand the cases and object spreads here. Not sure how much we can simplify this but do you think it'd help to use our arrayify
utility to basically remove the typeof checks? Although then again I'm not sure what types currentEntryPoint
could be of. Feel free to disregard!
): void { | ||
// BIG FAT NOTE: Order of insertion seems to matter here. If we insert the new files before the `currentEntrypoint`s, the Next.js dev server breaks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much cleaner!
Fixes #9067
The order in which we insert the SDK into the webpack entrypoints seems to matter in dev mode. Maybe the dev init script is somehow referenced by index. 🤷♂️
There is a slight nuanced consideration here in which the SDK may now be initialized too late to catch any errors generated by Next.js which we insert at the end only in dev mode.