-
-
Couldn't load subscription status.
- Fork 2.1k
perf: cache dynamic imports of nodes #10080
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
Changes from all commits
7a25baa
af097a9
ed474b0
20da030
e895f87
535c51f
7b61b69
cdc4e42
363142d
52806c6
5ed73b2
f7df598
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@sveltejs/kit': patch | ||
| --- | ||
|
|
||
| perf: cache dynamic imports of nodes |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,7 +44,7 @@ export function generate_manifest({ build_data, relative_path, routes }) { | |
| ); | ||
|
|
||
| /** @type {(path: string) => string} */ | ||
| const loader = (path) => `() => import('${path}')`; | ||
| const loader = (path) => `__memo(() => import('${path}'))`; | ||
|
|
||
| const assets = build_data.manifest_data.assets.map((asset) => asset.file); | ||
| if (build_data.service_worker) { | ||
|
|
@@ -65,8 +65,8 @@ export function generate_manifest({ build_data, relative_path, routes }) { | |
|
|
||
| // prettier-ignore | ||
| // String representation of | ||
| /** @type {import('@sveltejs/kit').SSRManifest} */ | ||
| return dedent` | ||
| /** @template {import('@sveltejs/kit').SSRManifest} T */ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it's correct to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's a hack to get VSCode to create a jump link for the SSRManifest. setting it as @type gave an error because the string is not of the type SSRManifest |
||
| const manifest_expr = dedent` | ||
| { | ||
| appDir: ${s(build_data.app_dir)}, | ||
| appPath: ${s(build_data.app_path)}, | ||
|
|
@@ -97,10 +97,26 @@ export function generate_manifest({ build_data, relative_path, routes }) { | |
| }).filter(Boolean).join(',\n')} | ||
| ], | ||
| matchers: async () => { | ||
| ${Array.from(matchers).map(type => `const { match: ${type} } = await import ('${(join_relative(relative_path, `/entries/matchers/${type}.js`))}')`).join('\n')} | ||
| ${Array.from( | ||
| matchers, | ||
| type => `const { match: ${type} } = await import ('${(join_relative(relative_path, `/entries/matchers/${type}.js`))}')` | ||
| ).join('\n')} | ||
| return { ${Array.from(matchers).join(', ')} }; | ||
| } | ||
| } | ||
| } | ||
| `; | ||
|
|
||
gtm-nayan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Memoize the loaders to prevent Node from doing unnecessary work | ||
| // on every dynamic import call | ||
| return dedent` | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small comment maybe why we need the memo |
||
| (() => { | ||
| function __memo(fn) { | ||
| let value; | ||
| return () => value ??= (value = fn()); | ||
| } | ||
|
|
||
| return ${manifest_expr} | ||
| })() | ||
| `; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,7 +84,12 @@ const encoder = new TextEncoder(); | |
| export function text(body, init) { | ||
| const headers = new Headers(init?.headers); | ||
| if (!headers.has('content-length')) { | ||
| headers.set('content-length', encoder.encode(body).byteLength.toString()); | ||
| const encoded = encoder.encode(body); | ||
| headers.set('content-length', encoded.byteLength.toString()); | ||
| return new Response(encoded, { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this a drive-by fix? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another perf thing to avoid duplicating the encoding work inside the Response constructor. |
||
| ...init, | ||
| headers | ||
| }); | ||
| } | ||
|
|
||
| return new Response(body, { | ||
|
|
||
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.
Small comment maybe why we memo the import
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.
Added a comment near the __memo declaration below. That should suffice.