Skip to content
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

fix(optimizer): ignore unknown deps in graph #6888

Merged
merged 1 commit into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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