Skip to content

Commit

Permalink
fix(optimizer): ignore unknown deps in graph
Browse files Browse the repository at this point in the history
  • Loading branch information
wmertens committed Sep 20, 2024
1 parent c48a23d commit ae09275
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 17 deletions.
5 changes: 5 additions & 0 deletions .changeset/quiet-grapes-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@builder.io/qwik': patch
---

fix(optimizer): ignore unknown deps in graph causing crashes during build
6 changes: 3 additions & 3 deletions e2e/qwik-cli-e2e/package.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"name": "qwik-cli-e2e",
"dependencies": {
"kleur": "4.1.5"
},
"private": true,
"scripts": {
"e2e": "vitest run --config=vite.config.ts",
"e2e:watch": "vitest watch --config=vite.config.ts"
},
"dependencies": {
"kleur": "4.1.5"
}
}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,8 @@
"serve.debug": "tsm --inspect-brk --conditions=development starters/dev-server.ts 3300",
"start": "concurrently \"npm:build.watch\" \"npm:tsc.watch\" -n build,tsc -c green,cyan",
"test": "pnpm build.full && pnpm test.unit && pnpm test.e2e",
"test.e2e-cli": "pnpm --filter qwik-cli-e2e e2e",
"test.e2e": "pnpm test.e2e.chromium && pnpm test.e2e.webkit",
"test.e2e-cli": "pnpm --filter qwik-cli-e2e e2e",
"test.e2e.chromium": "playwright test starters --browser=chromium --config starters/playwright.config.ts",
"test.e2e.chromium.debug": "PWDEBUG=1 playwright test starters --browser=chromium --config starters/playwright.config.ts",
"test.e2e.city": "playwright test starters/e2e/qwikcity --browser=chromium --config starters/playwright.config.ts",
Expand Down
26 changes: 15 additions & 11 deletions packages/qwik/src/optimizer/src/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,16 +261,21 @@ export function generateManifestFromBundles(
const buildPath = path.resolve(opts.rootDir, opts.outDir, 'build');
const canonPath = (p: string) =>
path.relative(buildPath, path.resolve(opts.rootDir, opts.outDir, p));
const getBundleName = (name: string) => {
const bundle = outputBundles[name];
if (!bundle) {
console.warn(`Client manifest generation: skipping external import "${name}"`);
return;
}
return canonPath(bundle.fileName);
};
// We need to find our QRL exports
const qrlNames = new Set([...segments.map((h) => h.name)]);
for (const outputBundle of Object.values(outputBundles)) {
if (outputBundle.type !== 'chunk') {
continue;
}
const bundleFileName = path.relative(
buildPath,
path.resolve(opts.outDir, outputBundle.fileName)
);
const bundleFileName = canonPath(outputBundle.fileName);

const bundle: QwikBundle = {
size: outputBundle.code.length,
Expand All @@ -296,19 +301,18 @@ export function generateManifestFromBundles(
}

const bundleImports = outputBundle.imports
// Tree shaking can maybe remove imports
// Tree shaking might remove imports
.filter((i) => outputBundle.code.includes(path.basename(i)))
.map((i) => canonPath(outputBundles[i].fileName || i));
.map((i) => getBundleName(i))
.filter(Boolean) as string[];
if (bundleImports.length > 0) {
bundle.imports = bundleImports;
}

const bundleDynamicImports = outputBundle.dynamicImports
.filter(
// Tree shaking can remove dynamic imports
(i) => outputBundle.code.includes(path.basename(i))
)
.map((i) => canonPath(outputBundles[i].fileName || i));
.filter((i) => outputBundle.code.includes(path.basename(i)))
.map((i) => getBundleName(i))
.filter(Boolean) as string[];
if (bundleDynamicImports.length > 0) {
bundle.dynamicImports = bundleDynamicImports;
}
Expand Down
8 changes: 6 additions & 2 deletions packages/qwik/src/optimizer/src/plugins/vite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,10 @@ export function convertManifestToBundleGraph(manifest: QwikManifest): QwikBundle
const map = new Map<string, { index: number; deps: Set<string> }>();
const clearTransitiveDeps = (parentDeps: Set<string>, seen: Set<string>, bundleName: string) => {
const bundle = graph[bundleName];
if (!bundle) {
// external dependency
return;
}
for (const dep of bundle.imports || []) {
if (parentDeps.has(dep)) {
parentDeps.delete(dep);
Expand All @@ -1151,7 +1155,7 @@ export function convertManifestToBundleGraph(manifest: QwikManifest): QwikBundle
const deps = new Set(bundle.imports);
for (const depName of deps) {
if (!graph[depName]) {
// weird but ok
// external dependency
continue;
}
clearTransitiveDeps(deps, new Set(), depName);
Expand All @@ -1161,7 +1165,7 @@ export function convertManifestToBundleGraph(manifest: QwikManifest): QwikBundle
// If we dynamically import a qrl segment that is not a handler, we'll probably need it soon
const dep = graph[depName];
if (!graph[depName]) {
// weird but ok
// external dependency
continue;
}
if (dep.isTask) {
Expand Down

0 comments on commit ae09275

Please sign in to comment.