drop dynamic import with `ssr: false` on server-side #32606

sokra · 2021-12-17T15:03:14Z

This drops the function passed to next/dynamic on server-side when ssr: false is statically found in the options object.

Improves build performance, reduces bundle size on the server-side and avoids touching client-only libraries at all on server-side.

Bug

Related issues linked using fixes #number
Integration tests added
Errors have helpful link attached, see contributing.md

Feature

Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
Related issues linked using fixes #number
Integration tests added
Documentation added
Telemetry added. In case of a feature if it's used or not.
Errors have helpful link attached, see contributing.md

Documentation / Examples

Make sure the linting passes by running yarn lint

kdy1 · 2022-01-10T10:36:55Z

packages/next-swc/crates/core/src/next_dynamic.rs

                    if expr.args.len() == 2 {
                        if let Expr::Object(ObjectLit {
                            props: options_props,
                            ..
                        }) = &*expr.args[1].expr
                        {
+                            for prop in options_props.iter() {
+                                if let Some(KeyValueProp { key, value }) =
+                                    prop.clone().prop().and_then(|p| p.key_value())


This will clone prop even if it's not required.

You can do

if let Some(KeyValueProp { key, value }) = prop.as_ref().and_then(|prop| match prop { PropOrSpread::Prop(prop) => match &**prop { Prop::KeyValue(kv) => Some(kv), _ => None, } _ => None, }) { }

instead

With nightly-only feature box_syntax,

You can do

if let PropOrSpread::Prop(box Prop::KeyValue(KeyValueProp { key, value })) = prop { }

but this is nightly-only.

You seem to generate these enum accessors for the ast nodes, e. g.

pub fn key_value(self) -> Option<KeyValueProp> // Returns Some if self is of variant KeyValue, and None otherwise.

In wonder why there isn't a ref variant to this, e. g.

pub fn key_value_ref(&self) -> Option<&KeyValueProp> // Returns Some if self is of variant KeyValue, and None otherwise.

I would like to write this nested if let match stuff as:

if let Some(KeyValueProp { key, value }) = try { prop.prop_ref()?.key_value_ref()? } {

or with Option::and_then instead of try blocks^^

For ref-based methods, I think it's good to have one. I'll implement it soon.

Done. swc-project/swc#3226

It can't be used with current code, because ast type definitions are modified recently.

You can rebase and use methods related to ref after I make a PR to update swc crates, or modify code after merging this PR.
Which do you prefer?
If you prefer the former, I'll make a upgrade PR asap.

I think this can be improved in follow up PRs.

packages/next-swc/crates/core/src/next_dynamic.rs

kdy1

LGTM. NIce work!

ijjk · 2022-01-11T12:19:29Z

Stats from current PR

Default Build (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	Change
buildDuration	16.8s	16.7s	-39ms
buildDurationCached	3.7s	3.7s	⚠️ +89ms
nodeModulesSize	355 MB	355 MB	⚠️ +7 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.555	3.601	⚠️ +0.05
/ avg req/sec	703.3	694.28	⚠️ -9.02
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.81	1.857	⚠️ +0.05
/error-in-render avg req/sec	1381.47	1346.09	⚠️ -35.38

Client Bundles (main, webpack, commons)

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	27.1 kB	27.1 kB	✓
webpack-HASH.js gzip	1.45 kB	1.45 kB	✓
Overall change	70.9 kB	70.9 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	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.74 kB	4.74 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	vercel/next.js perf/drop-nossr-dynamic-import-on-server	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	Change
index.html gzip	533 B	533 B	✓
link.html gzip	547 B	547 B	✓
withRouter.html gzip	528 B	528 B	✓
Overall change	1.61 kB	1.61 kB	✓

Default Build with SWC (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	Change
buildDuration	18.8s	18.6s	-192ms
buildDurationCached	3.8s	3.7s	-38ms
nodeModulesSize	355 MB	355 MB	⚠️ +7 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.676	3.598	-0.08
/ avg req/sec	680.13	694.76	+14.63
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	2.062	1.851	-0.21
/error-in-render avg req/sec	1212.36	1350.27	+137.91

Client Bundles (main, webpack, commons)

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.3 kB	42.3 kB	✓
main-HASH.js gzip	27.2 kB	27.2 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	71.1 kB	71.1 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	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.76 kB	4.76 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.1 kB	14.1 kB	✓

Client Build Manifests

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	Change
_buildManifest.js gzip	458 B	458 B	✓
Overall change	458 B	458 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js perf/drop-nossr-dynamic-import-on-server	Change
index.html gzip	532 B	532 B	✓
link.html gzip	545 B	545 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

Commit: 71f2aed

…into cms-builder-io-example * 'cms-builder-io-example' of github.com:teleaziz/next.js: (22 commits) drop dynamic import with `ssr: false` on server-side (vercel#32606) next-swc: fix ssg code elimination when used in render (vercel#32709) Add Caveats section to custom error page (vercel#33160) v12.0.8-canary.20 Allow dependencies to use environment variables in middlewares (vercel#33141) use a separate webpack runtime for middleware (vercel#33134) No info on environment variables in the src folder (vercel#33110) (vercel#33136) docs: minor text-copy cleanup (vercel#33120) Update swc (vercel#33063) Update next.config.js (vercel#33091) Adding Asset Component for Rich Text Renderer (vercel#32503) Update using-mdx.md (vercel#33077) v12.0.8-canary.19 Fix middleware at root in standalone mode (vercel#33053) Add util for generating new tests/error documents (vercel#33001) Remove extra config from tailwind example (vercel#33062) Fix link for Next.js Analytics in docs (vercel#33049) Bump `@vercel/nft` to 0.17.2 (vercel#33048) Update deployment documentation. (vercel#32006) v12.0.8-canary.18 ...

ijjk added created-by: Next.js team PRs by the Next.js team type: next labels Dec 17, 2021

This comment has been minimized.

Sign in to view

sokra marked this pull request as ready for review December 17, 2021 16:38

sokra requested review from huozhi, ijjk, shuding and timneutkens as code owners December 17, 2021 16:38

This comment has been minimized.

Sign in to view

kdy1 requested changes Jan 10, 2022

View reviewed changes

sokra added 2 commits January 10, 2022 15:02

drop dynamic import with ssr: false on server-side

41431ad

avoid cloning, use expr factory

b22fdc4

sokra force-pushed the perf/drop-nossr-dynamic-import-on-server branch from ce27a78 to b22fdc4 Compare January 10, 2022 14:50

This comment has been minimized.

Sign in to view

kdy1 approved these changes Jan 11, 2022

View reviewed changes

Merge branch 'canary' into perf/drop-nossr-dynamic-import-on-server

71f2aed

timneutkens merged commit 9836429 into canary Jan 11, 2022

timneutkens deleted the perf/drop-nossr-dynamic-import-on-server branch January 11, 2022 13:46

vercel locked as resolved and limited conversation to collaborators Feb 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drop dynamic import with `ssr: false` on server-side #32606

drop dynamic import with `ssr: false` on server-side #32606

sokra commented Dec 17, 2021 •

edited by timneutkens

This comment has been minimized.

This comment has been minimized.

kdy1 Jan 10, 2022

kdy1 Jan 10, 2022

kdy1 Jan 10, 2022

sokra Jan 10, 2022

sokra Jan 10, 2022 •

edited

kdy1 Jan 11, 2022

kdy1 Jan 11, 2022

sokra Jan 11, 2022

This comment has been minimized.

kdy1 left a comment

ijjk commented Jan 11, 2022

drop dynamic import with ssr: false on server-side #32606

drop dynamic import with ssr: false on server-side #32606

Conversation

sokra commented Dec 17, 2021 • edited by timneutkens

Bug

Feature

Documentation / Examples

This comment has been minimized.

This comment has been minimized.

kdy1 Jan 10, 2022

Choose a reason for hiding this comment

kdy1 Jan 10, 2022

Choose a reason for hiding this comment

kdy1 Jan 10, 2022

Choose a reason for hiding this comment

sokra Jan 10, 2022

Choose a reason for hiding this comment

sokra Jan 10, 2022 • edited

Choose a reason for hiding this comment

kdy1 Jan 11, 2022

Choose a reason for hiding this comment

kdy1 Jan 11, 2022

Choose a reason for hiding this comment

sokra Jan 11, 2022

Choose a reason for hiding this comment

This comment has been minimized.

kdy1 left a comment

Choose a reason for hiding this comment

ijjk commented Jan 11, 2022

Stats from current PR

drop dynamic import with `ssr: false` on server-side #32606

drop dynamic import with `ssr: false` on server-side #32606

sokra commented Dec 17, 2021 •

edited by timneutkens

sokra Jan 10, 2022 •

edited