Skip to content

Commit d2f7fba

Browse files
wmertenswmertens
committed
perf(bundle-graph): only include segments for dynamic imports
If the dev wants to include other dynamic imports, they can just make a qrl()
1 parent 28c6236 commit d2f7fba

File tree

8 files changed

+129
-125
lines changed

8 files changed

+129
-125
lines changed

‎packages/docs/src/routes/api/qwik-optimizer/api.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
}
3030
],
3131
"kind": "TypeAlias",
32-
"content": "A function that returns a map of bundle names to their dependencies.\n\n\n```typescript\nexport type BundleGraphAdder = (manifest: QwikManifest) => Record<string, {\n imports?: string[];\n dynamicImports?: string[];\n}>;\n```\n**References:** [QwikManifest](#qwikmanifest)",
32+
"content": "A function that returns a map of bundle names to their dependencies.\n\n\n```typescript\nexport type BundleGraphAdder = (manifest: QwikManifest) => Record<string, {\n imports?: string[];\n dynamicImports?: string[];\n hasSegments?: boolean;\n}>;\n```\n**References:** [QwikManifest](#qwikmanifest)",
3333
"editUrl": "https://github.com/QwikDev/qwik/tree/main/packages/qwik/src/optimizer/src/plugins/bundle-graph.ts",
3434
"mdFile": "qwik.bundlegraphadder.md"
3535
},
@@ -406,7 +406,7 @@
406406
}
407407
],
408408
"kind": "Interface",
409-
"content": "```typescript\nexport interface QwikBundle \n```\n\n\n<table><thead><tr><th>\n\nProperty\n\n\n</th><th>\n\nModifiers\n\n\n</th><th>\n\nType\n\n\n</th><th>\n\nDescription\n\n\n</th></tr></thead>\n<tbody><tr><td>\n\n[dynamicImports?](#)\n\n\n</td><td>\n\n\n</td><td>\n\nstring\\[\\]\n\n\n</td><td>\n\n_(Optional)_\n\n\n</td></tr>\n<tr><td>\n\n[hasSymbols?](#)\n\n\n</td><td>\n\n\n</td><td>\n\nboolean\n\n\n</td><td>\n\n_(Optional)_\n\n\n</td></tr>\n<tr><td>\n\n[imports?](#)\n\n\n</td><td>\n\n\n</td><td>\n\nstring\\[\\]\n\n\n</td><td>\n\n_(Optional)_\n\n\n</td></tr>\n<tr><td>\n\n[origins?](#)\n\n\n</td><td>\n\n\n</td><td>\n\nstring\\[\\]\n\n\n</td><td>\n\n_(Optional)_\n\n\n</td></tr>\n<tr><td>\n\n[size](#)\n\n\n</td><td>\n\n\n</td><td>\n\nnumber\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[symbols?](#)\n\n\n</td><td>\n\n\n</td><td>\n\nstring\\[\\]\n\n\n</td><td>\n\n_(Optional)_\n\n\n</td></tr>\n</tbody></table>",
409+
"content": "```typescript\nexport interface QwikBundle \n```\n\n\n<table><thead><tr><th>\n\nProperty\n\n\n</th><th>\n\nModifiers\n\n\n</th><th>\n\nType\n\n\n</th><th>\n\nDescription\n\n\n</th></tr></thead>\n<tbody><tr><td>\n\n[dynamicImports?](#)\n\n\n</td><td>\n\n\n</td><td>\n\nstring\\[\\]\n\n\n</td><td>\n\n_(Optional)_\n\n\n</td></tr>\n<tr><td>\n\n[hasSegments?](#)\n\n\n</td><td>\n\n\n</td><td>\n\nboolean\n\n\n</td><td>\n\n_(Optional)_\n\n\n</td></tr>\n<tr><td>\n\n[imports?](#)\n\n\n</td><td>\n\n\n</td><td>\n\nstring\\[\\]\n\n\n</td><td>\n\n_(Optional)_\n\n\n</td></tr>\n<tr><td>\n\n[origins?](#)\n\n\n</td><td>\n\n\n</td><td>\n\nstring\\[\\]\n\n\n</td><td>\n\n_(Optional)_\n\n\n</td></tr>\n<tr><td>\n\n[size](#)\n\n\n</td><td>\n\n\n</td><td>\n\nnumber\n\n\n</td><td>\n\n\n</td></tr>\n<tr><td>\n\n[symbols?](#)\n\n\n</td><td>\n\n\n</td><td>\n\nstring\\[\\]\n\n\n</td><td>\n\n_(Optional)_\n\n\n</td></tr>\n</tbody></table>",
410410
"editUrl": "https://github.com/QwikDev/qwik/tree/main/packages/qwik/src/optimizer/src/types.ts",
411411
"mdFile": "qwik.qwikbundle.md"
412412
},

‎packages/docs/src/routes/api/qwik-optimizer/index.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ export type BundleGraphAdder = (manifest: QwikManifest) => Record<
6262
{
6363
imports?: string[];
6464
dynamicImports?: string[];
65+
hasSegments?: boolean;
6566
}
6667
>;
6768
```
@@ -1272,7 +1273,7 @@ _(Optional)_
12721273
</td></tr>
12731274
<tr><td>
12741275

1275-
[hasSymbols?](#)
1276+
[hasSegments?](#)
12761277

12771278
</td><td>
12781279

‎packages/qwik/src/optimizer/src/api.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type { Plugin as Plugin_2 } from 'vite';
1010
export type BundleGraphAdder = (manifest: QwikManifest) => Record<string, {
1111
imports?: string[];
1212
dynamicImports?: string[];
13+
hasSegments?: boolean;
1314
}>;
1415

1516
// @public (undocumented)
@@ -170,7 +171,7 @@ export interface QwikBundle {
170171
// (undocumented)
171172
dynamicImports?: string[];
172173
// (undocumented)
173-
hasSymbols?: boolean;
174+
hasSegments?: boolean;
174175
// (undocumented)
175176
imports?: string[];
176177
// (undocumented)

‎packages/qwik/src/optimizer/src/manifest.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,19 +281,19 @@ export function generateManifestFromBundles(
281281
size: outputBundle.code.length,
282282
};
283283

284-
let hasSymbols = false;
284+
let hasSegments = false;
285285
for (const symbol of outputBundle.exports) {
286286
if (qrlNames.has(symbol)) {
287287
// When not minifying we see both the entry and the segment file
288288
// The segment file will only have 1 export, we want the entry
289289
if (!manifest.mapping[symbol] || outputBundle.exports.length !== 1) {
290-
hasSymbols = true;
290+
hasSegments = true;
291291
manifest.mapping[symbol] = bundleFileName;
292292
}
293293
}
294294
}
295-
if (hasSymbols) {
296-
bundle.hasSymbols = true;
295+
if (hasSegments) {
296+
bundle.hasSegments = true;
297297
}
298298

299299
const bundleImports = outputBundle.imports

‎packages/qwik/src/optimizer/src/plugins/bundle-graph.ts‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import type { QwikBundle, QwikBundleGraph, QwikManifest } from '../types';
88
*/
99
export type BundleGraphAdder = (
1010
manifest: QwikManifest
11-
) => Record<string, { imports?: string[]; dynamicImports?: string[] }>;
11+
) => Record<string, { imports?: string[]; dynamicImports?: string[]; hasSegments?: boolean }>;
1212

1313
const dynamicTag = '<dynamic>';
1414
/**
@@ -54,13 +54,30 @@ export function convertManifestToBundleGraph(
5454
}
5555
}
5656

57+
// Filter out external and non-segment dynamic imports
58+
for (const bundleName of Object.keys(graph)) {
59+
const bundle = graph[bundleName];
60+
const imports = bundle.imports?.filter((dep) => graph[dep]) || [];
61+
62+
// We only include dynamic imports that have qrl segments
63+
// If the dev wants to include other dynamic imports, they can just make a qrl()
64+
const dynamicImports = bundle.dynamicImports?.filter((dep) => graph[dep]?.hasSegments) || [];
65+
66+
// Overwrite so we don't mutate
67+
graph[bundleName] = {
68+
imports,
69+
dynamicImports,
70+
size: bundle.size,
71+
};
72+
}
73+
5774
// Remove unused bundles
5875
const notUsed = new Set(Object.keys(graph));
5976
for (const bundleName of Object.keys(graph)) {
60-
for (const dep of graph[bundleName].imports || []) {
77+
for (const dep of graph[bundleName].imports!) {
6178
notUsed.delete(dep);
6279
}
63-
for (const dep of graph[bundleName].dynamicImports || []) {
80+
for (const dep of graph[bundleName].dynamicImports!) {
6481
notUsed.delete(dep);
6582
}
6683
}
@@ -79,11 +96,7 @@ export function convertManifestToBundleGraph(
7996
seen: Set<string> = new Set()
8097
) => {
8198
const bundle = graph[bundleName];
82-
if (!bundle) {
83-
// external dependency
84-
return;
85-
}
86-
for (const dep of bundle.imports || []) {
99+
for (const dep of bundle.imports!) {
87100
if (parentDeps.has(dep)) {
88101
parentDeps.delete(dep);
89102
}
@@ -93,9 +106,6 @@ export function convertManifestToBundleGraph(
93106
}
94107
}
95108
};
96-
const withoutExternalDependencies = (imports: string[] | undefined) => {
97-
return imports?.filter((dep) => graph[dep]) || [];
98-
};
99109

100110
/**
101111
* First pass to collect minimal dependency lists and allocate space for dependencies. Minimal
@@ -104,11 +114,12 @@ export function convertManifestToBundleGraph(
104114
*/
105115
for (const bundleName of names) {
106116
const bundle = graph[bundleName];
107-
const deps = new Set(withoutExternalDependencies(bundle.imports));
117+
// external dependencies are not included in `graph`
118+
const deps = new Set(bundle.imports!);
108119
for (const depName of deps) {
109120
clearTransitiveDeps(deps, depName);
110121
}
111-
const dynDeps = new Set(withoutExternalDependencies(bundle.dynamicImports));
122+
const dynDeps = new Set(bundle.dynamicImports!);
112123
for (const depName of dynDeps) {
113124
clearTransitiveDeps(dynDeps, depName);
114125
}
@@ -130,11 +141,7 @@ export function convertManifestToBundleGraph(
130141

131142
// Second pass to set the dependency indices
132143
for (const bundleName of names) {
133-
const bundle = map.get(bundleName);
134-
if (!bundle) {
135-
console.warn(`Bundle ${bundleName} not found in the bundle graph.`);
136-
continue;
137-
}
144+
const bundle = map.get(bundleName)!;
138145
// eslint-disable-next-line prefer-const
139146
let { index, deps } = bundle;
140147
index++;

‎packages/qwik/src/optimizer/src/plugins/bundle-graph.unit.ts‎

Lines changed: 91 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,27 @@ describe('convertManifestToBundleGraph', () => {
1515
},
1616
'static-dep.js': {
1717
size: 0,
18-
dynamicImports: ['@other', 'transitive-dep.js', 'dynamic-dep.js'],
18+
dynamicImports: ['@other', 'transitive-dep.js', 'dynamic-dep.js', 'no-symbols.js'],
1919
},
2020
'dynamic-dep.js': {
2121
size: 0,
2222
imports: ['static-dep.js', 'transitive-dep.js', '@external-dep'],
23+
dynamicImports: ['has-a-symbol.js', 'no-symbols.js'],
24+
hasSegments: true,
2325
},
2426
'transitive-dep.js': {
2527
size: 0,
28+
hasSegments: true,
2629
},
2730
'not-used.js': {
2831
size: 0,
2932
},
3033
'has-a-symbol.js': {
3134
size: 0,
32-
symbols: ['sym2'],
35+
hasSegments: true,
36+
},
37+
'no-symbols.js': {
38+
size: 0,
3339
},
3440
} as Record<string, QwikBundle>,
3541
mapping: { sym1: 'app.js', sym2: 'has-a-symbol.js' },
@@ -39,34 +45,71 @@ describe('convertManifestToBundleGraph', () => {
3945
} as QwikManifest;
4046

4147
test('trivial example', () => {
42-
expect(convertManifestToBundleGraph(fakeManifest)).toMatchInlineSnapshot(`
43-
[
44-
"app.js",
45-
2,
46-
"static-dep.js",
47-
-1,
48-
5,
49-
"dynamic-dep.js",
50-
2,
51-
8,
52-
"transitive-dep.js",
53-
"has-a-symbol.js",
54-
"sym1",
55-
0,
56-
"sym2",
57-
9,
58-
]
59-
`);
48+
expect(convertManifestToBundleGraph(fakeManifest)).toEqual([
49+
'app.js', // 0
50+
2,
51+
'static-dep.js', // 2
52+
-1,
53+
5,
54+
// doesn't list 8 because it's also statically imported by dynamic-dep.js
55+
'dynamic-dep.js', // 5
56+
2,
57+
10,
58+
-1,
59+
11,
60+
'transitive-dep.js', // 10
61+
'has-a-symbol.js', // 11
62+
'sym1', // 13
63+
0,
64+
'sym2', // 15
65+
11,
66+
]);
67+
});
68+
69+
test('empty', () => {
70+
expect(convertManifestToBundleGraph({} as any)).toEqual([]);
71+
});
72+
73+
test('simple file set', () => {
74+
const manifest = {
75+
bundles: {
76+
'a.js': {
77+
size: 0,
78+
imports: ['b.js'],
79+
dynamicImports: ['c.js'],
80+
},
81+
'b.js': {
82+
size: 0,
83+
dynamicImports: ['c.js'],
84+
},
85+
'c.js': {
86+
size: 0,
87+
hasSegments: true,
88+
},
89+
} as Record<string, QwikBundle>,
90+
mapping: {},
91+
} as QwikManifest;
92+
expect(convertManifestToBundleGraph(manifest)).toEqual([
93+
'a.js', // 0
94+
4,
95+
-1,
96+
7,
97+
'b.js', // 4
98+
-1,
99+
7,
100+
'c.js', // 7
101+
]);
60102
});
103+
61104
test('adder', () => {
62105
expect(
63106
convertManifestToBundleGraph(
64107
fakeManifest,
65108
new Set([
66-
(_manifest) => {
109+
(manifest) => {
67110
return {
68111
// Remove dynamic imports from dynamic-dep.js
69-
'dynamic-dep.js': { dynamicImports: [] },
112+
'dynamic-dep.js': { ...manifest.bundles['dynamic-dep.js'], dynamicImports: [] },
70113
};
71114
},
72115
(_manifest) => {
@@ -80,27 +123,24 @@ describe('convertManifestToBundleGraph', () => {
80123
},
81124
])
82125
)
83-
).toMatchInlineSnapshot(`
84-
[
85-
"app.js",
86-
2,
87-
"static-dep.js",
88-
-1,
89-
7,
90-
6,
91-
"dynamic-dep.js",
92-
"transitive-dep.js",
93-
"has-a-symbol.js",
94-
"sym1",
95-
0,
96-
"sym2",
97-
8,
98-
"dashboard/",
99-
2,
100-
-1,
101-
7,
102-
]
103-
`);
126+
).toEqual([
127+
'app.js', // 0
128+
2,
129+
'static-dep.js', // 2
130+
-1,
131+
5,
132+
'dynamic-dep.js', // 5
133+
2,
134+
8,
135+
'transitive-dep.js', // 8
136+
'has-a-symbol.js', // 9
137+
'sym1', // 11
138+
0,
139+
'sym2', // 13
140+
9,
141+
'dashboard/', // 15
142+
2,
143+
]);
104144
});
105145

106146
test(`works`, () => {
@@ -116,35 +156,27 @@ describe('convertManifestToBundleGraph', () => {
116156
expect(convertManifestToBundleGraph(manifest)).toMatchInlineSnapshot(`
117157
[
118158
"app.js",
119-
30,
120-
31,
159+
22,
160+
23,
121161
-1,
122162
19,
123163
"app.tsx_App_component_div_div_div_div_div_div_button_onClick_1_WC1qsF4RHoU.js",
124164
0,
125165
"app.tsx_App_component_div_div_div_div_div_div_button_onClick_2_cibDwmrlmRI.js",
126166
0,
127167
"app.tsx_App_component_div_div_div_div_div_div_button_onClick_3_dHjr9JWbW34.js",
128-
30,
168+
22,
129169
"app.tsx_App_component_div_div_div_div_div_div_button_onClick_4_5fYbrS6ABNA.js",
130-
30,
170+
22,
131171
"app.tsx_App_component_div_div_div_div_div_div_button_onClick_5_rpMfjSetIeI.js",
132-
30,
172+
22,
133173
"app.tsx_App_component_div_div_div_div_div_div_button_onClick_wEyctjlC58Q.js",
134174
0,
135175
"app.tsx_App_component_div_table_tbody_tr_td_a_onClick_DDeCLEw4BYU.js",
136-
30,
176+
22,
137177
"app.tsx_App_component_jn5XSz7NZ88.js",
138-
30,
139-
31,
140-
-1,
141-
5,
142-
7,
143-
9,
144-
11,
145-
13,
146-
15,
147-
17,
178+
22,
179+
23,
148180
"core.js",
149181
"preload-helper.js",
150182
"root.js",

0 commit comments

Comments
 (0)