-
Notifications
You must be signed in to change notification settings - Fork 30.3k
Turbopack: selective reads of defined env vars in module analysis #88759
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
base: canary
Are you sure you want to change the base?
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
e6f15e9 to
4096331
Compare
8c6302c to
2494303
Compare
Equivalent keys
Failing test suitesCommit: 377d927 | About building and testing Next.js
Expand output● next-config-ts-node-api-esm › should be able to use Node.js API (ESM)
Expand output● next-config-ts-top-level-await-esm › should support top-level await (ESM)
Expand output● Basic CSS Module Support › production mode › should have compiled successfully ● Basic CSS Module Support › production mode › should've emitted a single CSS file ● Basic CSS Module Support › production mode › should've injected the CSS on server render ● 3rd Party CSS Module Support › production mode › should have compiled successfully ● 3rd Party CSS Module Support › production mode › should've emitted a single CSS file ● 3rd Party CSS Module Support › production mode › should've injected the CSS on server render ● Has CSS Module in computed styles in Development › should have CSS for page ● Has CSS Module in computed styles in Production › production mode › should have compiled successfully ● Has CSS Module in computed styles in Production › production mode › should have CSS for page ● Can hot reload CSS Module without losing state › should update CSS color without remounting ● Valid CSS Module Usage from within node_modules › production mode › should have compiled successfully ● Valid CSS Module Usage from within node_modules › production mode › should've prerendered with relevant data ● Valid CSS Module Usage from within node_modules › production mode › should've emitted a single CSS file ● CSS Module Composes Usage (Basic) › production mode › should have compiled successfully ● CSS Module Composes Usage (Basic) › production mode › should've emitted a single CSS file ● Dynamic Route CSS Module Usage › production mode › should have compiled successfully ● Dynamic Route CSS Module Usage › production mode › should apply styles correctly ● Dynamic Route CSS Module Usage › production mode › should've emitted a single CSS file ● Catch-all Route CSS Module Usage › production mode › should have compiled successfully ● Catch-all Route CSS Module Usage › production mode › should apply styles correctly ● Catch-all Route CSS Module Usage › production mode › should've emitted a single CSS file ● cssmodules-pure-no-check usage › should have compiled successfully ● cssmodules-pure-no-check usage › should apply styles correctly ● cssmodules-pure-no-check usage › should've emitted a CSS file
Expand output● Image Optimizer › Server support for headers in next.config.js › production mode › should set max-age header ● Image Optimizer › Server support for headers in next.config.js › production mode › should not set max-age header when not matching next.config.js ● Image Optimizer › dev support next.config.js cloudinary loader › should 404 when loader is not default ● Image Optimizer › images.unoptimized in next.config.js › should 404 when unoptimized ● Image Optimizer › External rewrite support with for serving static content in images › production mode › should return response when image is served from an external rewrite
Expand output● Image Loader Config new › development mode - component › should work with loaderFile config ● Image Loader Config new › development mode - component › should work with loader prop ● Image Loader Config new › production mode - component › should work with loaderFile config ● Image Loader Config new › production mode - component › should work with loader prop ● Image Loader Config new › development mode - getImageProps › should work with loaderFile config ● Image Loader Config new › development mode - getImageProps › should work with loader prop ● Image Loader Config new › production mode - getImageProps › should work with loaderFile config ● Image Loader Config new › production mode - getImageProps › should work with loader prop
Expand output● Basics › default setting dev › should only render once in SSR ● Basics › default setting dev › hydrates correctly for normal page ● Basics › default setting dev › useId() values should match on hydration ● Basics › default setting dev › should contain dynamicIds in next data for dynamic imports ● Basics › production mode › default setting prod › should only render once in SSR ● Basics › production mode › default setting prod › no warnings for image related link props ● Basics › production mode › default setting prod › hydrates correctly for normal page ● Basics › production mode › default setting prod › useId() values should match on hydration ● Basics › production mode › default setting prod › should contain dynamicIds in next data for dynamic imports ● Concurrent mode in the experimental-edge runtime dev › flushes styled-jsx styles as the page renders ● Concurrent mode in the experimental-edge runtime dev › › should not have the initial route announced ● production mode › Concurrent mode in the experimental-edge runtime prod › flushes styled-jsx styles as the page renders ● production mode › Concurrent mode in the experimental-edge runtime prod › › should not have the initial route announced ● production mode › Concurrent mode in the experimental-edge runtime prod › should not have invalid config warning ● Concurrent mode in the nodejs runtime dev › flushes styled-jsx styles as the page renders ● Concurrent mode in the nodejs runtime dev › › should not have the initial route announced ● production mode › Concurrent mode in the nodejs runtime prod › flushes styled-jsx styles as the page renders ● production mode › Concurrent mode in the nodejs runtime prod › › should not have the initial route announced ● production mode › Concurrent mode in the nodejs runtime prod › should not have invalid config warning
Expand output● next-config-ts-import-json-esm › should support import json (ESM)
Expand output● Can hot reload CSS without losing state › should update CSS color without remounting ● Has CSS in computed styles in Development › should have CSS for page ● Body is not hidden when unused in Development › should have body visible ● Body is not hidden when broken in Development › should have body visible ● React Lifecyce Order (dev) › should have the correct color on mount after navigation
Expand output● Middleware development errors › when middleware is removed › sends response correctly ● Middleware development errors › when middleware is removed and re-added › sends response correctly ● Middleware development errors › when middleware is added › sends response correctly ● Middleware development errors › when matcher is added › sends response correctly
Expand output● Side-effect imports with noUncheckedSideEffectImports › Should build without typescript errors
Expand output● Read-only source HMR › should detect changes to a page ● Read-only source HMR › should handle page deletion and subsequent recreation ● Read-only source HMR › should detect a new page
Expand output● dynamic-css-client-navigation react lazy edge › should not remove style when navigating from static imported component to react lazy at runtime edge ● dynamic-css-client-navigation react lazy nodejs › should not remove style when navigating from static imported component to react lazy at runtime nodejs
Expand output● next-server-nft › with output:standalone › should not trace too many files in next-server.js.nft.json ● next-server-nft › default mode › should not include .next directory in traces despite dynamic fs operations ● next-server-nft › default mode › should not trace too many files in next-minimal-server.js.nft.json
Expand output● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID › should append dpl query to all assets correctly for / ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID › should append dpl query to all assets correctly for /pages-edge ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID › should append dpl query to all assets correctly for /from-app ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID › should append dpl query to all assets correctly for /from-app/edge ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID › should have deployment id env available ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID › should have deployment id env available ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID › should contain deployment id in prefetch request ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID › should contain deployment id in RSC payload request headers ● deployment-id-handling enabled with CUSTOM_DEPLOYMENT_ID › should append dpl query to all assets correctly for / ● deployment-id-handling enabled with CUSTOM_DEPLOYMENT_ID › should append dpl query to all assets correctly for /pages-edge ● deployment-id-handling enabled with CUSTOM_DEPLOYMENT_ID › should append dpl query to all assets correctly for /from-app ● deployment-id-handling enabled with CUSTOM_DEPLOYMENT_ID › should append dpl query to all assets correctly for /from-app/edge ● deployment-id-handling enabled with CUSTOM_DEPLOYMENT_ID › should have deployment id env available ● deployment-id-handling enabled with CUSTOM_DEPLOYMENT_ID › should have deployment id env available ● deployment-id-handling enabled with CUSTOM_DEPLOYMENT_ID › should contain deployment id in prefetch request ● deployment-id-handling enabled with CUSTOM_DEPLOYMENT_ID › should contain deployment id in RSC payload request headers ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID and runtimeServerDeploymentId › should append dpl query to all assets correctly for / ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID and runtimeServerDeploymentId › should append dpl query to all assets correctly for /pages-edge ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID and runtimeServerDeploymentId › should append dpl query to all assets correctly for /from-app ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID and runtimeServerDeploymentId › should append dpl query to all assets correctly for /from-app/edge ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID and runtimeServerDeploymentId › should have deployment id env available ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID and runtimeServerDeploymentId › should have deployment id env available ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID and runtimeServerDeploymentId › should contain deployment id in prefetch request ● deployment-id-handling enabled with NEXT_DEPLOYMENT_ID and runtimeServerDeploymentId › should contain deployment id in RSC payload request headers ● deployment-id-handling disabled › should not append dpl query to all assets for / ● deployment-id-handling disabled › should not append dpl query to all assets for /pages-edge ● deployment-id-handling disabled › should not append dpl query to all assets for /from-app ● deployment-id-handling disabled › should not append dpl query to all assets for /from-app/edge
Expand output● debug-build-paths › default fixture › explicit path formats › should build single page with pages/ prefix ● debug-build-paths › default fixture › explicit path formats › should build multiple pages routes ● debug-build-paths › default fixture › explicit path formats › should build dynamic route with literal [slug] path ● debug-build-paths › default fixture › glob pattern matching › should match app and pages routes with glob patterns ● debug-build-paths › default fixture › glob pattern matching › should match nested routes with app/blog/**/page.tsx pattern ● debug-build-paths › default fixture › glob pattern matching › should match dynamic routes with glob before brackets like app/**/[slug]/page.tsx ● debug-build-paths › default fixture › glob pattern matching › should match hybrid pattern with literal [slug] and glob ** ● debug-build-paths › default fixture › glob pattern matching › should match multiple app routes with explicit patterns ● debug-build-paths › default fixture › glob pattern matching › should exclude paths matching negation patterns ● debug-build-paths › default fixture › glob pattern matching › should exclude dynamic route paths with negation ● debug-build-paths › default fixture › glob pattern matching › should support multiple negation patterns ● debug-build-paths › default fixture › glob pattern matching › should build everything except excluded paths when only negation patterns are provided ● debug-build-paths › default fixture › typechecking with debug-build-paths › should skip typechecking for excluded app routes ● debug-build-paths › with-compile-error fixture › should skip compilation of excluded routes with compile errors
Expand output● segment cache (deployment skew) › does not crash when prefetching a dynamic, non-PPR page on a different deployment ● segment cache (deployment skew) › does not crash when prefetching a dynamic, non-PPR page on a different deployment ● segment cache (deployment skew) › does not crash when prefetching a static page on a different deployment ● segment cache (deployment skew) › does not crash when prefetching a static page on a different deployment ● Test suite failed to run
Expand output● Link Component with Encoding › spaces › should have correct query on SSR ● Link Component with Encoding › spaces › should have correct query on Router#push ● Link Component with Encoding › spaces › should have correct query on simple client-side ● Link Component with Encoding › percent › should have correct query on SSR ● Link Component with Encoding › percent › should have correct query on Router#push ● Link Component with Encoding › percent › should have correct query on simple client-side ● Link Component with Encoding › forward slash › should have correct query on SSR ● Link Component with Encoding › forward slash › should have correct query on Router#push ● Link Component with Encoding › forward slash › should have correct query on simple client-side ● Link Component with Encoding › double quote › should have correct query on SSR ● Link Component with Encoding › double quote › should have correct query on Router#push ● Link Component with Encoding › double quote › should have correct query on simple client-side ● Link Component with Encoding › colon › should have correct query on SSR ● Link Component with Encoding › colon › should have correct query on Router#push ● Link Component with Encoding › colon › should have correct query on simple client-side ● Link Component with Encoding › colon › should have correct parsing of url query params
Expand output● GS(S)P Redirect Support › development mode › should apply temporary redirect when visited directly for GSSP page ● GS(S)P Redirect Support › development mode › should apply permanent redirect when visited directly for GSSP page ● GS(S)P Redirect Support › development mode › should apply statusCode 301 redirect when visited directly for GSSP page ● GS(S)P Redirect Support › development mode › should apply statusCode 303 redirect when visited directly for GSSP page ● GS(S)P Redirect Support › development mode › should apply redirect when fallback GSP page is visited directly (internal dynamic) ● GS(S)P Redirect Support › development mode › should apply redirect when fallback blocking GSP page is visited directly (internal dynamic) ● GS(S)P Redirect Support › development mode › should apply redirect when fallback blocking GSP page is visited directly (internal dynamic) second visit ● GS(S)P Redirect Support › development mode › should apply redirect when fallback blocking GSP page is visited directly (internal dynamic) with revalidate ● GS(S)P Redirect Support › development mode › should apply redirect when fallback blocking GSP page is visited directly (internal dynamic) with revalidate second visit ● GS(S)P Redirect Support › development mode › should apply redirect when fallback GSP page is visited directly (internal normal) ● GS(S)P Redirect Support › development mode › should apply redirect when fallback GSP page is visited directly (external) ● GS(S)P Redirect Support › development mode › should apply redirect when fallback GSP page is visited directly (external domain) ● GS(S)P Redirect Support › development mode › should apply redirect when fallback GSSP page is visited directly (external domain) ● GS(S)P Redirect Support › development mode › should apply redirect when GSSP page is navigated to client-side (internal dynamic) ● GS(S)P Redirect Support › development mode › should apply redirect when GSSP page is navigated to client-side (internal normal) ● GS(S)P Redirect Support › development mode › should apply redirect when GSSP page is navigated to client-side (external) ● GS(S)P Redirect Support › development mode › should apply redirect when GSP page is navigated to client-side (internal) ● GS(S)P Redirect Support › development mode › should apply redirect when GSP page is navigated to client-side (external) ● GS(S)P Redirect Support › development mode › should not replace history of the origin page when GSSP page is navigated to client-side (internal normal) ● GS(S)P Redirect Support › development mode › should not replace history of the origin page when GSSP page is navigated to client-side (external) ● GS(S)P Redirect Support › development mode › should not replace history of the origin page when GSP page is navigated to client-side (internal) ● GS(S)P Redirect Support › development mode › should not replace history of the origin page when GSP page is navigated to client-side (external)
Expand output● router.isReady with appGip › development mode › isReady should be true immediately for pages without getStaticProps ● router.isReady with appGip › development mode › isReady should be true immediately for pages without getStaticProps, with query ● router.isReady with appGip › development mode › isReady should be true immediately for getStaticProps page without query ● router.isReady with appGip › development mode › isReady should be true after query update for getStaticProps page with query
Expand output● Image Component basePath Tests › development mode › should load the images ● Image Component basePath Tests › development mode › should update the image on src change ● Image Component basePath Tests › development mode › should work with layout-fixed so resizing window does not resize image ● Image Component basePath Tests › development mode › should work with layout-intrinsic so resizing window maintains image aspect ratio ● Image Component basePath Tests › development mode › should work with layout-responsive so resizing window maintains image aspect ratio ● Image Component basePath Tests › development mode › should work with layout-fill to fill the parent but NOT stretch with viewport ● Image Component basePath Tests › development mode › should work with layout-fill to fill the parent and stretch with viewport ● Image Component basePath Tests › development mode › should work with sizes and automatically use layout-responsive ● Image Component basePath Tests › development mode › should show missing src error ● Image Component basePath Tests › development mode › should show invalid src error ● Image Component basePath Tests › development mode › should show invalid src error when protocol-relative ● Image Component basePath Tests › development mode › should correctly ignore prose styles
Expand output● preferred-region › should return success from Node.js API route ● preferred-region › should return success from Edge API route
Expand output● app dir - not found with nested layouts › should render the custom not-found page when notFound() is thrown from a page |
Merging this PR will improve performance by ×6.7
Performance Changes
Comparing Footnotes
|
| } | ||
| } | ||
|
|
||
| #[turbo_tasks::value(transparent, cell = "keyed")] |
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.
| return Ok(()); | ||
| } | ||
| } | ||
| if let Some(name) = var.get_definable_name() { |
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.
| analysis: &mut AnalyzeEcmascriptModuleResultBuilder, | ||
| ) -> Result<()> { | ||
| if let Some(prop) = prop.as_str() { | ||
| let prop_seg = DefinableNameSegment::Name(prop.into()); |
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.
Stats from current PR✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
|
4096331 to
448c79a
Compare
2494303 to
6282cc0
Compare
448c79a to
97c8270
Compare
c634085 to
aa12d3e
Compare
97c8270 to
5b92c6a
Compare
aa12d3e to
461c2bb
Compare
5b92c6a to
aef5e3c
Compare
aef5e3c to
c62192a
Compare
461c2bb to
84bcebc
Compare
391f2e7 to
5d08678
Compare
84bcebc to
088bbd8
Compare
5d08678 to
57e790c
Compare
| for segment in name.iter().rev() { | ||
| segments.push(DefinableNameSegmentRef::Name(segment)); | ||
| } |
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.
| for segment in name.iter().rev() { | |
| segments.push(DefinableNameSegmentRef::Name(segment)); | |
| } | |
| segments.extend(name.iter().rev().map(DefinableNameSegmentRef::Name)); |
| for segment in name.iter().rev() { | ||
| segments.push(DefinableNameSegmentRef::Name(segment)); | ||
| } |
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.
| for segment in name.iter().rev() { | |
| segments.push(DefinableNameSegmentRef::Name(segment)); | |
| } | |
| segments.extend(name.iter().rev().map(DefinableNameSegmentRef::Name)); |
| let prop_seg = DefinableNameSegment::Name(prop.into()); | ||
|
|
||
| let references = state.free_var_references.get(&prop_seg); | ||
| let has_member = state.free_var_references_members.contains_key(prop).await?; |
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.
Have you measured the initial build time impact? This adds an await into the hotpath.
| for (key, _) in self.0.iter() { | ||
| if let Some(DefinableNameSegment::Name(name)) = key | ||
| .iter() | ||
| .rfind(|segment| matches!(segment, DefinableNameSegment::Name(_))) |
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.
Why filter on Name here?
And why is members() needed in the first place? Why can't we just check FreeVarReferences.contains_key()?
57e790c to
377d927
Compare

What?
Use selective reads to read defined env vars in module analysis.
This allows to add/remove env vars without invalidating all modules.