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.

Sign up for GitHub

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

Jump to bottom

Strictly match next/dist in externals configuration #9956

Merged
merged 2 commits into from Jan 6, 2020
Merged

Strictly match next/dist in externals configuration #9956

merged 2 commits into from Jan 6, 2020

Conversation

felixmosh
Copy link
Contributor

@felixmosh felixmosh commented Jan 5, 2020 •
edited

such as i18next, next-i18next, react-i18next to be external
image
fixes #9022

@felixmosh felixmosh changed the title Allow libs that ends with next/dist Allow libs that ends with next/dist to be external in server bundle Jan 5, 2020
@ijjk
Copy link
Member

ijjk commented Jan 5, 2020

Stats from current PR

Default Server Mode
General Overall increase ⚠️
zeit/next.js canary felixmosh/next.js fix-9022 Change
buildDuration 13.5s 13.6s ⚠️ +143ms
nodeModulesSize 48.9 MB 48.9 MB ⚠️ +10 B
Client Bundles (main, webpack, commons)
zeit/next.js canary felixmosh/next.js fix-9022 Change
main-HASH.js gzip 5.13 kB 5.13 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
4952ddcd88e7..54d3.js gzip 4.68 kB 4.68 kB ✓
commons.HASH.js gzip 4.06 kB 4.06 kB ✓
de003c3a9d30..4cf7.js gzip 13.7 kB 13.7 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 67.5 kB 67.5 kB ✓
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary felixmosh/next.js fix-9022 Change
main-HASH.module.js gzip 4.19 kB 4.19 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
4952ddcd88e7..dule.js gzip 5.56 kB 5.56 kB ✓
de003c3a9d30..dule.js gzip 12.5 kB 12.5 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 62.1 kB 62.1 kB ✓
Legacy Client Bundles (polyfills)
zeit/next.js canary felixmosh/next.js fix-9022 Change
polyfills-HASH.js gzip 4.76 kB 4.76 kB ✓
Overall change 4.76 kB 4.76 kB ✓
Client Pages
zeit/next.js canary felixmosh/next.js fix-9022 Change
_app.js gzip 1.33 kB 1.33 kB ✓
_error.js gzip 4.07 kB 4.07 kB ✓
hooks.js gzip 779 B 779 B ✓
index.js gzip 222 B 222 B ✓
link.js gzip 2.9 kB 2.9 kB ✓
routerDirect.js gzip 283 B 283 B ✓
withRouter.js gzip 282 B 282 B ✓
Overall change 9.87 kB 9.87 kB ✓
Client Pages Modern
zeit/next.js canary felixmosh/next.js fix-9022 Change
_app.module.js gzip 757 B 757 B ✓
_error.module.js gzip 3.06 kB 3.06 kB ✓
hooks.module.js gzip 371 B 371 B ✓
index.module.js gzip 212 B 212 B ✓
link.module.js gzip 2.47 kB 2.47 kB ✓
routerDirect..dule.js gzip 273 B 273 B ✓
withRouter.m..dule.js gzip 272 B 272 B ✓
Overall change 7.41 kB 7.41 kB ✓
Client Build Manifests
zeit/next.js canary felixmosh/next.js fix-9022 Change
_buildManifest.js gzip 61 B 61 B ✓
_buildManife..dule.js gzip 61 B 61 B ✓
Overall change 122 B 122 B ✓
Rendered Page Sizes
zeit/next.js canary felixmosh/next.js fix-9022 Change
index.html gzip 1.02 kB 1.02 kB ✓
link.html gzip 1.03 kB 1.03 kB ✓
withRouter.html gzip 1.02 kB 1.02 kB ✓
Overall change 3.07 kB 3.07 kB ✓
Serverless Mode
General Overall increase ⚠️
zeit/next.js canary felixmosh/next.js fix-9022 Change
buildDuration 13.9s 14s ⚠️ +141ms
nodeModulesSize 48.9 MB 48.9 MB ⚠️ +10 B
Client Bundles (main, webpack, commons)
zeit/next.js canary felixmosh/next.js fix-9022 Change
main-HASH.js gzip 5.13 kB 5.13 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
4952ddcd88e7..54d3.js gzip 4.68 kB 4.68 kB ✓
commons.HASH.js gzip 4.06 kB 4.06 kB ✓
de003c3a9d30..4cf7.js gzip 13.7 kB 13.7 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 67.5 kB 67.5 kB ✓
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary felixmosh/next.js fix-9022 Change
main-HASH.module.js gzip 4.19 kB 4.19 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
4952ddcd88e7..dule.js gzip 5.56 kB 5.56 kB ✓
de003c3a9d30..dule.js gzip 12.5 kB 12.5 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 62.1 kB 62.1 kB ✓
Legacy Client Bundles (polyfills)
zeit/next.js canary felixmosh/next.js fix-9022 Change
polyfills-HASH.js gzip 4.76 kB 4.76 kB ✓
Overall change 4.76 kB 4.76 kB ✓
Client Pages
zeit/next.js canary felixmosh/next.js fix-9022 Change
_app.js gzip 1.33 kB 1.33 kB ✓
_error.js gzip 4.07 kB 4.07 kB ✓
hooks.js gzip 779 B 779 B ✓
index.js gzip 222 B 222 B ✓
link.js gzip 2.9 kB 2.9 kB ✓
routerDirect.js gzip 283 B 283 B ✓
withRouter.js gzip 282 B 282 B ✓
Overall change 9.87 kB 9.87 kB ✓
Client Pages Modern
zeit/next.js canary felixmosh/next.js fix-9022 Change
_app.module.js gzip 757 B 757 B ✓
_error.module.js gzip 3.06 kB 3.06 kB ✓
hooks.module.js gzip 371 B 371 B ✓
index.module.js gzip 212 B 212 B ✓
link.module.js gzip 2.47 kB 2.47 kB ✓
routerDirect..dule.js gzip 273 B 273 B ✓
withRouter.m..dule.js gzip 272 B 272 B ✓
Overall change 7.41 kB 7.41 kB ✓
Client Build Manifests
zeit/next.js canary felixmosh/next.js fix-9022 Change
_buildManifest.js gzip 61 B 61 B ✓
_buildManife..dule.js gzip 61 B 61 B ✓
Overall change 122 B 122 B ✓
Serverless bundles
zeit/next.js canary felixmosh/next.js fix-9022 Change
_error.js gzip 77.8 kB 77.8 kB ✓
hooks.html gzip 1.06 kB 1.06 kB ✓
index.js gzip 78 kB 78 kB ✓
link.js gzip 80.4 kB 80.4 kB ✓
routerDirect.js gzip 78.1 kB 78.1 kB ✓
withRouter.js gzip 78.1 kB 78.1 kB ✓
Overall change 393 kB 393 kB ✓

Commit: 7289e1f

@timneutkens
Copy link
Member

I did some investigating on #9022 and this does not fix the case that is described in the initial issue as that one is related to how React is resolved. However I'd argue that the case described in the initial issue is a wrong project structure (deps not available) and that what you've fixed here is more important 👍

@felixmosh
Copy link
Contributor Author

I had the same error as #9022 when updating to ^9.06, as I've wrote in the comment #9022 (comment), it is an issue of combination of 2 things, usage of sym-link & the fact that these libs had multiple instance in side server bundle...

I've tested, if one of the conditions is not apply, the bug is not reproduces.
When I've fixed the next regex, even sym-link started to work...

@timneutkens
Copy link
Member

Could you add a test for this? Then we can ensure it keeps working.

@felixmosh
Copy link
Contributor Author

felixmosh commented Jan 6, 2020 •
edited

Sure, @timneutkens can you suggest where to start? in which folder? how I can add an external dependency that has *next/dist in tests?

@baldurh
Copy link

baldurh commented Jan 6, 2020

I can confirm this PR does not fix #9022 in my case. See my #9022 (comment).

@timneutkens I wouldn’t say this is a result of a wrong project structure, but rather an unusual project structure. However, I’m not the only one as can be seen in the original issue.

@Timer Timer added this to the 9.2.0 milestone Jan 6, 2020
@felixmosh
Copy link
Contributor Author

I can confirm this PR does not fix #9022 in my case. See my #9022 (comment).

@timneutkens I wouldn’t say this is a result of a wrong project structure, but rather an unusual project structure. However, I’m not the only one as can be seen in the original issue.

I've double checked, it solves the issue that I had.
I didn't had weird structure, but a mono-repo with usage of sym-link for building the production build & usage of i18next that got bundled in this case inside the server bundles.

@timneutkens
Copy link
Member

@felixmosh your case is fine and this PR will definitely fix it. Baldur's case is that there is no node_modules folder outside of the subdirectory and that causes React to not be resolved. Then an alias was added to next.config.js but that doesn't work as expected. Particularly what changed that caused that issue in particular is yarn PnP support.

Note that if you have a Node.js project and you would require the file it would break in exactly the same way, hence why I'm saying the project structure is wrong.

@baldurh
Copy link

baldurh commented Jan 6, 2020

@timneutkens good point about Node.js. I guess you are right 😅

@Timer Timer changed the title Allow libs that ends with next/dist to be external in server bundle Strictly match next/dist in externals configuration Jan 6, 2020
Timer
Timer approved these changes Jan 6, 2020
View reviewed changes
Copy link
Member

@Timer Timer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll handle the tests, thanks!

@ijjk
Copy link
Member

ijjk commented Jan 6, 2020

Stats from current PR

Default Server Mode
General Overall increase ⚠️
zeit/next.js canary felixmosh/next.js fix-9022 Change
buildDuration 13.1s 13.1s ⚠️ +51ms
nodeModulesSize 48.9 MB 48.9 MB ⚠️ +10 B
Client Bundles (main, webpack, commons)
zeit/next.js canary felixmosh/next.js fix-9022 Change
main-HASH.js gzip 5.13 kB 5.13 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
4952ddcd88e7..54d3.js gzip 4.68 kB 4.68 kB ✓
commons.HASH.js gzip 4.06 kB 4.06 kB ✓
de003c3a9d30..4cf7.js gzip 13.7 kB 13.7 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 67.5 kB 67.5 kB ✓
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary felixmosh/next.js fix-9022 Change
main-HASH.module.js gzip 4.19 kB 4.19 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
4952ddcd88e7..dule.js gzip 5.56 kB 5.56 kB ✓
de003c3a9d30..dule.js gzip 12.5 kB 12.5 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 62.1 kB 62.1 kB ✓
Legacy Client Bundles (polyfills)
zeit/next.js canary felixmosh/next.js fix-9022 Change
polyfills-HASH.js gzip 4.76 kB 4.76 kB ✓
Overall change 4.76 kB 4.76 kB ✓
Client Pages
zeit/next.js canary felixmosh/next.js fix-9022 Change
_app.js gzip 1.33 kB 1.33 kB ✓
_error.js gzip 4.07 kB 4.07 kB ✓
hooks.js gzip 779 B 779 B ✓
index.js gzip 222 B 222 B ✓
link.js gzip 2.9 kB 2.9 kB ✓
routerDirect.js gzip 283 B 283 B ✓
withRouter.js gzip 282 B 282 B ✓
Overall change 9.87 kB 9.87 kB ✓
Client Pages Modern
zeit/next.js canary felixmosh/next.js fix-9022 Change
_app.module.js gzip 757 B 757 B ✓
_error.module.js gzip 3.06 kB 3.06 kB ✓
hooks.module.js gzip 371 B 371 B ✓
index.module.js gzip 212 B 212 B ✓
link.module.js gzip 2.47 kB 2.47 kB ✓
routerDirect..dule.js gzip 273 B 273 B ✓
withRouter.m..dule.js gzip 272 B 272 B ✓
Overall change 7.41 kB 7.41 kB ✓
Client Build Manifests
zeit/next.js canary felixmosh/next.js fix-9022 Change
_buildManifest.js gzip 61 B 61 B ✓
_buildManife..dule.js gzip 61 B 61 B ✓
Overall change 122 B 122 B ✓
Rendered Page Sizes
zeit/next.js canary felixmosh/next.js fix-9022 Change
index.html gzip 1.02 kB 1.02 kB ✓
link.html gzip 1.03 kB 1.03 kB ✓
withRouter.html gzip 1.02 kB 1.02 kB ✓
Overall change 3.07 kB 3.07 kB ✓
Serverless Mode
General Overall increase ⚠️
zeit/next.js canary felixmosh/next.js fix-9022 Change
buildDuration 13.7s 13.6s -42ms
nodeModulesSize 48.9 MB 48.9 MB ⚠️ +10 B
Client Bundles (main, webpack, commons)
zeit/next.js canary felixmosh/next.js fix-9022 Change
main-HASH.js gzip 5.13 kB 5.13 kB ✓
webpack-HASH.js gzip 746 B 746 B ✓
4952ddcd88e7..54d3.js gzip 4.68 kB 4.68 kB ✓
commons.HASH.js gzip 4.06 kB 4.06 kB ✓
de003c3a9d30..4cf7.js gzip 13.7 kB 13.7 kB ✓
framework.HASH.js gzip 39.1 kB 39.1 kB ✓
Overall change 67.5 kB 67.5 kB ✓
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary felixmosh/next.js fix-9022 Change
main-HASH.module.js gzip 4.19 kB 4.19 kB ✓
webpack-HASH..dule.js gzip 746 B 746 B ✓
4952ddcd88e7..dule.js gzip 5.56 kB 5.56 kB ✓
de003c3a9d30..dule.js gzip 12.5 kB 12.5 kB ✓
framework.HA..dule.js gzip 39.1 kB 39.1 kB ✓
Overall change 62.1 kB 62.1 kB ✓
Legacy Client Bundles (polyfills)
zeit/next.js canary felixmosh/next.js fix-9022 Change
polyfills-HASH.js gzip 4.76 kB 4.76 kB ✓
Overall change 4.76 kB 4.76 kB ✓
Client Pages
zeit/next.js canary felixmosh/next.js fix-9022 Change
_app.js gzip 1.33 kB 1.33 kB ✓
_error.js gzip 4.07 kB 4.07 kB ✓
hooks.js gzip 779 B 779 B ✓
index.js gzip 222 B 222 B ✓
link.js gzip 2.9 kB 2.9 kB ✓
routerDirect.js gzip 283 B 283 B ✓
withRouter.js gzip 282 B 282 B ✓
Overall change 9.87 kB 9.87 kB ✓
Client Pages Modern
zeit/next.js canary felixmosh/next.js fix-9022 Change
_app.module.js gzip 757 B 757 B ✓
_error.module.js gzip 3.06 kB 3.06 kB ✓
hooks.module.js gzip 371 B 371 B ✓
index.module.js gzip 212 B 212 B ✓
link.module.js gzip 2.47 kB 2.47 kB ✓
routerDirect..dule.js gzip 273 B 273 B ✓
withRouter.m..dule.js gzip 272 B 272 B ✓
Overall change 7.41 kB 7.41 kB ✓
Client Build Manifests
zeit/next.js canary felixmosh/next.js fix-9022 Change
_buildManifest.js gzip 61 B 61 B ✓
_buildManife..dule.js gzip 61 B 61 B ✓
Overall change 122 B 122 B ✓
Serverless bundles
zeit/next.js canary felixmosh/next.js fix-9022 Change
_error.js gzip 77.8 kB 77.8 kB ✓
hooks.html gzip 1.05 kB 1.05 kB ✓
index.js gzip 78 kB 78 kB ✓
link.js gzip 80.4 kB 80.4 kB ✓
routerDirect.js gzip 78.1 kB 78.1 kB ✓
withRouter.js gzip 78.1 kB 78.1 kB ✓
Overall change 393 kB 393 kB ✓

Commit: ae44d77

@Timer Timer merged commit dea80b8 into vercel:canary Jan 6, 2020
@Timer Timer mentioned this pull request Jan 6, 2020
Merged
@Timer
Copy link
Member

Timer commented Jan 6, 2020

Test added in #9968

@felixmosh felixmosh deleted the fix-9022 branch January 6, 2020 20:54
@Timer
Copy link
Member

Timer commented Jan 6, 2020

Released as 9.1.8-canary.3.

@m-spyratos
Copy link

m-spyratos commented Feb 11, 2020 •
edited

@tim-phillips I'm watching #9022 because I've setup my project following the sample NextJS with Firebase Hosting. As soon as I install React Bootstrap or React Material I get this error cause I end up with multiple react instances.

I would suggest to keep #9022 open or at least update the documentation with a better solution. It's a very common scenario libraries like the above to be used alongside React and the proposed Firebase file structure doesn't allow this at the moment.

@snyk-bot snyk-bot mentioned this pull request Feb 11, 2020
Merged
@vercel vercel locked as resolved and limited conversation to collaborators Jan 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@timneutkens timneutkens timneutkens approved these changes

@Timer Timer Timer approved these changes

@ijjk ijjk Awaiting requested review from ijjk ijjk is a code owner

@lfades lfades Awaiting requested review from lfades

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
9.2.0
Development

Successfully merging this pull request may close these issues.

Invalid hook call in 9.0.6
6 participants
@felixmosh @ijjk @timneutkens @baldurh @Timer @m-spyratos