fix(next-swc/ssg): less aggressive exports drop #36199
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kwonoj
requested review from
timneutkens,
ijjk,
shuding and
huozhi
as code owners
Stats from current PRDefault Build (Decrease detected ✓)General
Page Load Tests Overall decrease
|
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | Change | |
|---|---|---|---|
| / failed reqs | 0 | 0 | ✓ |
| / total time (seconds) | 3.243 | 3.197 | -0.05 |
| / avg req/sec | 770.87 | 781.88 | +11.01 |
| /error-in-render failed reqs | 0 | 0 | ✓ |
| /error-in-render total time (seconds) | 1.368 | 1.477 | |
| /error-in-render avg req/sec | 1827.79 | 1692.23 |
Client Bundles (main, webpack)
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | Change | |
|---|---|---|---|
| 925.HASH.js gzip | 179 B | 179 B | ✓ |
| framework-HASH.js gzip | 42 kB | 42 kB | ✓ |
| main-HASH.js gzip | 28.5 kB | 28.5 kB | ✓ |
| webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
| Overall change | 72.1 kB | 72.1 kB | ✓ |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | Change | |
|---|---|---|---|
| polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
| Overall change | 31 kB | 31 kB | ✓ |
Client Pages
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | Change | |
|---|---|---|---|
| _app-HASH.js gzip | 1.36 kB | 1.36 kB | ✓ |
| _error-HASH.js gzip | 192 B | 192 B | ✓ |
| amp-HASH.js gzip | 309 B | 309 B | ✓ |
| css-HASH.js gzip | 327 B | 327 B | ✓ |
| dynamic-HASH.js gzip | 3.04 kB | 3.04 kB | ✓ |
| head-HASH.js gzip | 351 B | 351 B | ✓ |
| hooks-HASH.js gzip | 920 B | 920 B | ✓ |
| image-HASH.js gzip | 5.74 kB | 5.74 kB | ✓ |
| index-HASH.js gzip | 263 B | 263 B | ✓ |
| link-HASH.js gzip | 2.36 kB | 2.36 kB | ✓ |
| routerDirect..HASH.js gzip | 320 B | 320 B | ✓ |
| script-HASH.js gzip | 392 B | 392 B | ✓ |
| withRouter-HASH.js gzip | 319 B | 319 B | ✓ |
| 85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
| Overall change | 16 kB | 16 kB | ✓ |
Client Build Manifests
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | Change | |
|---|---|---|---|
| _buildManifest.js gzip | 461 B | 461 B | ✓ |
| Overall change | 461 B | 461 B | ✓ |
Rendered Page Sizes
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | Change | |
|---|---|---|---|
| index.html gzip | 532 B | 532 B | ✓ |
| link.html gzip | 546 B | 546 B | ✓ |
| withRouter.html gzip | 526 B | 526 B | ✓ |
| Overall change | 1.6 kB | 1.6 kB | ✓ |
Default Build with SWC (Increase detected ⚠️ )
General
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | Change | |
|---|---|---|---|
| buildDuration | 18.3s | 18s | -240ms |
| buildDurationCached | 5.5s | 6.2s | |
| nodeModulesSize | 485 MB | 485 MB | ✓ |
Page Load Tests Overall increase ✓
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | Change | |
|---|---|---|---|
| / failed reqs | 0 | 0 | ✓ |
| / total time (seconds) | 3.203 | 3.176 | -0.03 |
| / avg req/sec | 780.49 | 787.23 | +6.74 |
| /error-in-render failed reqs | 0 | 0 | ✓ |
| /error-in-render total time (seconds) | 1.356 | 1.354 | 0 |
| /error-in-render avg req/sec | 1843.03 | 1845.78 | +2.75 |
Client Bundles (main, webpack)
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | Change | |
|---|---|---|---|
| 925.HASH.js gzip | 178 B | 178 B | ✓ |
| framework-HASH.js gzip | 42.3 kB | 42.3 kB | ✓ |
| main-HASH.js gzip | 28.9 kB | 28.9 kB | ✓ |
| webpack-HASH.js gzip | 1.45 kB | 1.45 kB | ✓ |
| Overall change | 72.8 kB | 72.8 kB | ✓ |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | Change | |
|---|---|---|---|
| polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
| Overall change | 31 kB | 31 kB | ✓ |
Client Pages
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | Change | |
|---|---|---|---|
| _app-HASH.js gzip | 1.35 kB | 1.35 kB | ✓ |
| _error-HASH.js gzip | 179 B | 179 B | ✓ |
| amp-HASH.js gzip | 313 B | 313 B | ✓ |
| css-HASH.js gzip | 325 B | 325 B | ✓ |
| dynamic-HASH.js gzip | 3.02 kB | 3.02 kB | ✓ |
| head-HASH.js gzip | 351 B | 351 B | ✓ |
| hooks-HASH.js gzip | 921 B | 921 B | ✓ |
| image-HASH.js gzip | 5.78 kB | 5.78 kB | ✓ |
| index-HASH.js gzip | 261 B | 261 B | ✓ |
| link-HASH.js gzip | 2.44 kB | 2.44 kB | ✓ |
| routerDirect..HASH.js gzip | 322 B | 322 B | ✓ |
| script-HASH.js gzip | 393 B | 393 B | ✓ |
| withRouter-HASH.js gzip | 317 B | 317 B | ✓ |
| 85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
| Overall change | 16.1 kB | 16.1 kB | ✓ |
Client Build Manifests
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | Change | |
|---|---|---|---|
| _buildManifest.js gzip | 457 B | 457 B | ✓ |
| Overall change | 457 B | 457 B | ✓ |
Rendered Page Sizes
| vercel/next.js canary | kwonoj/next.js fix-less-aggressive-ssr-drop | 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 | ✓ |
ijjk
reviewed
Apr 15, 2022
| } | ||
|
|
||
| if let Pat::Ident(id) = &d.decls[0].name { | ||
| let ssg_exports = &["getStaticProps", "getStaticPaths", "getServerSideProps"]; |
There was a problem hiding this comment.
Should we define this at the top-level so it can be shared instead of re-defining it each time or could we re-use the is_data_identifier method here?
There was a problem hiding this comment.
Probably yes. I wrote PR to gut check if this sounds correct idea first before trying to refactor.
ijjk
reviewed
Apr 15, 2022
kwonoj
force-pushed
the
fix-less-aggressive-ssr-drop
branch
from
3d3b061
to
ab0896a
Compare
Added one simple test cases to verify its behavior. |
timneutkens
approved these changes
Apr 19, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR attempts to fix #31855, by loosening conditions to determine what to drop when swc runs transform. Currently, it drops all the export declaration if it's being referenced only in getstatic* in local scope. But as
exportimplies, there's no guarantee given export will be used in other modules even if it's not being used in local scope. PR tries to not to drop exports declarations as much if it's not being used locally other than getstatic*. This makes dropping bit ineffecient, but as long as we can't cross-ref across modules that's something unavoidable in my opinion.I don't think implementation itself is quite acceptable: probably need review & revise to the logics.
Bug
Attempt to close Exported members that are used in SSR/SSG/ISG page function are undefined on the client (Attempted import error: '..' is not exported from '..' (imported as '..').) #31855
Related issues linked using
fixes #numberIntegration tests added
Errors have helpful link attached, see
contributing.mdFeature
fixes #numbercontributing.mdDocumentation / Examples
yarn lint