-
Notifications
You must be signed in to change notification settings - Fork 27.8k
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
feat(server): isolate component modules #29565
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Some tricky conflicts going on. I just got a comment from @sokra in another thread, but many of the failing test errors seem to be Webpack-related, would appreciate a first-glance review for sure. |
Failing test suitesCommit: e2ff1d0 test/development/acceptance/ReactRefreshLogBox.test.ts
Expand output● ReactRefreshLogBox › module init error not shown
|
This comment has been minimized.
This comment has been minimized.
This source for the module in this error overlay test changes from `(eval)` to `(unknown)` when injected by Jest, and is purely cosmetic.
Stats from current PRDefault Build (Decrease detected ✓)General Overall increase
|
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
buildDuration | 13.9s | 14.1s | |
buildDurationCached | 3.1s | 3.2s | |
nodeModulesSize | 354 MB | 354 MB |
Page Load Tests Overall decrease ⚠️
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 2.872 | 7.934 | |
/ avg req/sec | 870.34 | 315.12 | |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.325 | 8.527 | |
/error-in-render avg req/sec | 1886.33 | 293.2 |
Client Bundles (main, webpack, commons)
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42.2 kB | 42.2 kB | ✓ |
main-HASH.js gzip | 27.2 kB | 27.2 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71 kB | 71 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.37 kB | 1.37 kB | ✓ |
_error-HASH.js gzip | 194 B | 194 B | ✓ |
amp-HASH.js gzip | 312 B | 312 B | ✓ |
css-HASH.js gzip | 326 B | 326 B | ✓ |
dynamic-HASH.js gzip | 2.37 kB | 2.37 kB | ✓ |
head-HASH.js gzip | 350 B | 350 B | ✓ |
hooks-HASH.js gzip | 919 B | 919 B | ✓ |
image-HASH.js gzip | 4.7 kB | 4.7 kB | ✓ |
index-HASH.js gzip | 263 B | 263 B | ✓ |
link-HASH.js gzip | 2.13 kB | 2.13 kB | ✓ |
routerDirect..HASH.js gzip | 321 B | 321 B | ✓ |
script-HASH.js gzip | 383 B | 383 B | ✓ |
withRouter-HASH.js gzip | 318 B | 318 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.1 kB | 14.1 kB | ✓ |
Client Build Manifests
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
_buildManifest.js gzip | 459 B | 459 B | ✓ |
Overall change | 459 B | 459 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
index.html gzip | 530 B | 530 B | ✓ |
link.html gzip | 544 B | 544 B | ✓ |
withRouter.html gzip | 525 B | 525 B | ✓ |
Overall change | 1.6 kB | 1.6 kB | ✓ |
Default Build with SWC (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
buildDuration | 15.7s | 15.9s | |
buildDurationCached | 3.1s | 3.2s | |
nodeModulesSize | 354 MB | 354 MB |
Page Load Tests Overall decrease ⚠️
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 2.903 | 8.468 | |
/ avg req/sec | 861.1 | 295.24 | |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.281 | 7.904 | |
/error-in-render avg req/sec | 1952.07 | 316.29 |
Client Bundles (main, webpack, commons)
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42.3 kB | 42.3 kB | ✓ |
main-HASH.js gzip | 27.3 kB | 27.3 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.2 kB | 71.2 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.35 kB | 1.35 kB | ✓ |
_error-HASH.js gzip | 180 B | 180 B | ✓ |
amp-HASH.js gzip | 305 B | 305 B | ✓ |
css-HASH.js gzip | 321 B | 321 B | ✓ |
dynamic-HASH.js gzip | 2.36 kB | 2.36 kB | ✓ |
head-HASH.js gzip | 342 B | 342 B | ✓ |
hooks-HASH.js gzip | 906 B | 906 B | ✓ |
image-HASH.js gzip | 4.72 kB | 4.72 kB | ✓ |
index-HASH.js gzip | 256 B | 256 B | ✓ |
link-HASH.js gzip | 2.19 kB | 2.19 kB | ✓ |
routerDirect..HASH.js gzip | 314 B | 314 B | ✓ |
script-HASH.js gzip | 375 B | 375 B | ✓ |
withRouter-HASH.js gzip | 309 B | 309 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14 kB | 14 kB | ✓ |
Client Build Manifests
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
_buildManifest.js gzip | 458 B | 458 B | ✓ |
Overall change | 458 B | 458 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | ctjlewis/next.js isolate-component-requires | Change | |
---|---|---|---|
index.html gzip | 531 B | 531 B | ✓ |
link.html gzip | 544 B | 544 B | ✓ |
withRouter.html gzip | 526 B | 526 B | ✓ |
Overall change | 1.6 kB | 1.6 kB | ✓ |
Pretty tough hit to render speed. |
df8579c
to
47e5ebe
Compare
e078ebe
to
6b863fe
Compare
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.
Closing as stale, feel free to open a new discussion if this is still a concern!
WIP to resolve issues encountered when adding global shims to an application. Because Next apps currently share context with the Next server itself, it is possible (and trivial) to write application code that breaks the server and prevents a perfectly valid component from rendering, i.e., you can run it with
node .next/server/pages/_app.js
and see the component render without issue, but the Next server itself will throw as a result of conflicts with application code.This feature aims to isolate component module contexts using the Node.js
vm
module and prevent this possibility (and also better containerize Next applications). In general, it is safe to say we do not want application code to ever touch the Next server context, as they truly are separate processes. It is probably worth considering a review of other places where application code is naivelyrequire
'd and replacing those calls withisolateRequire(...)
, but it seems likenext/server/require.ts : requirePage()
is the main concern.See: Agoric/dapp-card-store#37
Bug
fixes #number
contributing.md
Feature
fixes #number
contributing.md
Documentation / Examples