From ac118d2d2a39e13f9860eba5f86d51c408a5186f Mon Sep 17 00:00:00 2001 From: Wout Mertens Date: Thu, 28 Nov 2024 13:14:16 +0100 Subject: [PATCH 1/4] fix(core): throw when importing twice this should make it easier to detect build misconfigurations --- packages/qwik-router/src/ssg/worker-thread.ts | 6 ++++++ packages/qwik/src/core/index.ts | 13 +++++++++++++ starters/dev-server.ts | 5 ++++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/qwik-router/src/ssg/worker-thread.ts b/packages/qwik-router/src/ssg/worker-thread.ts index 884a85b1ae3..830bb38c31e 100644 --- a/packages/qwik-router/src/ssg/worker-thread.ts +++ b/packages/qwik-router/src/ssg/worker-thread.ts @@ -14,6 +14,9 @@ import type { } from './types'; export async function workerThread(sys: System) { + // Special case: we allow importing qwik again in the same process, it's ok because we just needed the serializer + // TODO: remove this once we have vite environment API and no longer need the serializer separately + delete (globalThis as any).__qwik; const ssgOpts = sys.getOptions(); const pendingPromises = new Set>(); @@ -42,6 +45,9 @@ export async function workerThread(sys: System) { } export async function createSingleThreadWorker(sys: System) { + // Special case: we allow importing qwik again in the same process, it's ok because we just needed the serializer + // TODO: remove this once we have vite environment API and no longer need the serializer separately + delete (globalThis as any).__qwik; const ssgOpts = sys.getOptions(); const pendingPromises = new Set>(); diff --git a/packages/qwik/src/core/index.ts b/packages/qwik/src/core/index.ts index f89906281d4..df095a0ed87 100644 --- a/packages/qwik/src/core/index.ts +++ b/packages/qwik/src/core/index.ts @@ -1,3 +1,16 @@ +////////////////////////////////////////////////////////////////////////////////////////// +// Protect against duplicate imports +////////////////////////////////////////////////////////////////////////////////////////// +import { version } from './version'; +if ((globalThis as any).__qwik) { + console.error( + `==============================================\n` + + `Qwik version ${(globalThis as any).__qwik} already imported while importing ${version}. Verify external vs bundled imports etc. This can lead to issues due to duplicated shared structures.\n` + + `==============================================\n` + ); +} +(globalThis as any).__qwik = version; + ////////////////////////////////////////////////////////////////////////////////////////// // Developer Core API ////////////////////////////////////////////////////////////////////////////////////////// diff --git a/starters/dev-server.ts b/starters/dev-server.ts index d8f99d1024e..eb214b9547b 100644 --- a/starters/dev-server.ts +++ b/starters/dev-server.ts @@ -271,7 +271,8 @@ async function routerApp( appDir: string, ) { const ssrPath = join(appDir, "server", `${qwikRouterVirtualEntry}.js`); - + // it's ok in the devserver to import core multiple times + (globalThis as any).__qwik = null; const mod = await import(file(ssrPath)); const router: any = mod.router; router(req, res, () => { @@ -289,6 +290,8 @@ async function ssrApp( manifest: QwikManifest, ) { const ssrPath = join(appDir, "server", "entry.ssr.js"); + // it's ok in the devserver to import core multiple times + (globalThis as any).__qwik = null; const mod = await import(file(ssrPath)); const render: Render = mod.default ?? mod.render; From 8688da1f1ab8558f6bf908fe72b0dd71e317fe14 Mon Sep 17 00:00:00 2001 From: Wout Mertens Date: Thu, 28 Nov 2024 13:22:49 +0100 Subject: [PATCH 2/4] perf(qwik): remove vendor scan at build start It reads all packages at startup. Instead, there's now an error if you externalize a qwik package during a server build. --- .changeset/lemon-dingos-dance.md | 5 + packages/docs/vite.config.ts | 3 + .../qwik/src/optimizer/src/plugins/vite.ts | 185 ++++++++---------- starters/dev-server.ts | 15 +- 4 files changed, 92 insertions(+), 116 deletions(-) create mode 100644 .changeset/lemon-dingos-dance.md diff --git a/.changeset/lemon-dingos-dance.md b/.changeset/lemon-dingos-dance.md new file mode 100644 index 00000000000..ee7569dd198 --- /dev/null +++ b/.changeset/lemon-dingos-dance.md @@ -0,0 +1,5 @@ +--- +'@builder.io/qwik': minor +--- + +BREAKING: (slightly) Qwik will no longer scan all modules at build start to detect Qwik modules. Instead, a runtime check is done to prevent duplicate core imports. If you get a runtime error, you need to fix your build settings so they don't externalize qwik-related libraries. diff --git a/packages/docs/vite.config.ts b/packages/docs/vite.config.ts index eb05c450371..7af02c1e298 100644 --- a/packages/docs/vite.config.ts +++ b/packages/docs/vite.config.ts @@ -137,6 +137,9 @@ export default defineConfig(async () => { 'algoliasearch', '@algolia/autocomplete-core/dist/esm/reshape', 'algoliasearch/dist/algoliasearch-lite.esm.browser', + 'qwik-image', + '@modular-forms/qwik', + '@qwik-ui/headless', ], }, diff --git a/packages/qwik/src/optimizer/src/plugins/vite.ts b/packages/qwik/src/optimizer/src/plugins/vite.ts index 79181cfc8ed..9b04541c257 100644 --- a/packages/qwik/src/optimizer/src/plugins/vite.ts +++ b/packages/qwik/src/optimizer/src/plugins/vite.ts @@ -5,7 +5,6 @@ import type { GlobalInjections, Optimizer, OptimizerOptions, - OptimizerSystem, QwikManifest, TransformModule, } from '../types'; @@ -25,7 +24,6 @@ import { type NormalizedQwikPluginOptions, type QwikBuildMode, type QwikBuildTarget, - type QwikPackages, type QwikPluginOptions, } from './plugin'; import { createRollupError, normalizeRollupOutputOptions } from './rollup'; @@ -96,7 +94,6 @@ export function qwikVite(qwikViteOpts: QwikVitePluginOptions = {}): any { async config(viteConfig, viteEnv) { await qwikPlugin.init(); - const sys = qwikPlugin.getSys(); const path = qwikPlugin.getPath(); let target: QwikBuildTarget; @@ -149,8 +146,6 @@ export function qwikVite(qwikViteOpts: QwikVitePluginOptions = {}): any { if (input && typeof input === 'string') { input = [input]; } - const shouldFindVendors = - !qwikViteOpts.disableVendorScan && (target !== 'lib' || viteCommand === 'serve'); viteAssetsDir = viteConfig.build?.assetsDir; const useAssetsDir = target === 'client' && !!viteAssetsDir && viteAssetsDir !== '_astro'; const pluginOpts: QwikPluginOptions = { @@ -208,8 +203,6 @@ export function qwikVite(qwikViteOpts: QwikVitePluginOptions = {}): any { clientDevInput = qwikPlugin.normalizePath(clientDevInput); } - const vendorRoots = shouldFindVendors ? await findQwikRoots(sys, sys.cwd()) : []; - const vendorIds = vendorRoots.map((v) => v.id); const isDevelopment = buildMode === 'development'; const qDevKey = 'globalThis.qDev'; const qTestKey = 'globalThis.qTest'; @@ -221,17 +214,11 @@ export function qwikVite(qwikViteOpts: QwikVitePluginOptions = {}): any { const updatedViteConfig: UserConfig = { ssr: { - noExternal: [ - QWIK_CORE_ID, - QWIK_CORE_INTERNAL_ID, - QWIK_CORE_SERVER, - QWIK_BUILD_ID, - ...vendorIds, - ], + noExternal: [QWIK_CORE_ID, QWIK_CORE_INTERNAL_ID, QWIK_CORE_SERVER, QWIK_BUILD_ID], }, envPrefix: ['VITE_', 'PUBLIC_'], resolve: { - dedupe: [...DEDUPE, ...vendorIds], + dedupe: [...DEDUPE], conditions: buildMode === 'production' && target === 'client' ? ['min'] : [], alias: { '@builder.io/qwik': '@qwik.dev/core', @@ -264,8 +251,6 @@ export function qwikVite(qwikViteOpts: QwikVitePluginOptions = {}): any { QWIK_JSX_DEV_RUNTIME_ID, QWIK_BUILD_ID, QWIK_CLIENT_MANIFEST_ID, - // Sadly we can't specify **/*.qwik.*, so we need to specify each one - ...vendorIds, // v1 imports, they are removed during transform but vite doesn't know that '@builder.io/qwik', '@builder.io/qwik-city', @@ -653,7 +638,87 @@ export function qwikVite(qwikViteOpts: QwikVitePluginOptions = {}): any { }, } as const satisfies VitePlugin; - return [vitePluginPre, vitePluginPost]; + return [vitePluginPre, vitePluginPost, checkExternals()]; +} + +/** + * This plugin checks for external dependencies that should be included in the server bundle, + * because they use Qwik. If they are not included, the optimizer won't process them, and there will + * be two instances of Qwik Core loaded. + */ +async function checkExternals() { + let fs: typeof import('fs').promises; + let path: typeof import('path'); + try { + fs = await import('node:fs').then((m) => m.promises); + path = await import('node:path'); + } catch { + // We can't do anything if we can't import fs and path + return; + } + const seen: Set = new Set(); + let rootDir: string; + const core2 = '@qwik-dev/core'; + const core1 = '@builder.io/qwik'; + async function isQwikDep(dep: string, dir: string) { + while (dir) { + const pkg = path.join(dir, 'node_modules', dep, 'package.json'); + try { + await fs.access(pkg); + const data = await fs.readFile(pkg, { + encoding: 'utf-8', + }); + // any mention of lowercase qwik in the package.json is enough + const json = JSON.parse(data); + if ( + json.qwik || + json.dependencies?.[core2] || + json.peerDependencies?.[core2] || + json.dependencies?.[core1] || + json.peerDependencies?.[core1] + ) { + return true; + } + return false; + } catch { + //empty + } + const nextRoot = path.dirname(dir); + if (nextRoot === dir) { + break; + } + dir = nextRoot; + } + return false; + } + + return { + name: 'checkQwikExternals', + enforce: 'pre', + configResolved: (config) => { + rootDir = config.root; + }, + resolveId: { + order: 'pre', + async handler(source, importer, options) { + if (source.startsWith('node:') || seen.has(source)) { + return; + } + // technically we should check for each importer, but this is ok + seen.add(source); + const result = await this.resolve(source, importer, { ...options, skipSelf: true }); + if (result?.external) { + // For Qwik files, check if they should be externalized + if (await isQwikDep(source, importer ? path.dirname(importer) : rootDir)) { + // TODO link to docs + throw new Error( + `\n==============\n${source} is being treated as an external dependency, but it should be included in the server bundle, because it uses Qwik.\nPlease add the package to "ssr.noExternal" in the Vite config. \n==============` + ); + } + } + }, + }, + } as const satisfies VitePlugin; } const ANSI_COLOR = { @@ -715,90 +780,6 @@ export async function render(document, rootNode, opts) { }`; } -async function findDepPkgJsonPath(sys: OptimizerSystem, dep: string, parent: string) { - const fs: typeof import('fs') = await sys.dynamicImport('node:fs'); - let root = parent; - while (root) { - const pkg = sys.path.join(root, 'node_modules', dep, 'package.json'); - try { - await fs.promises.access(pkg); - // use 'node:fs' version to match 'vite:resolve' and avoid realpath.native quirk - // https://github.com/sveltejs/vite-plugin-svelte/issues/525#issuecomment-1355551264 - return fs.promises.realpath(pkg); - } catch { - //empty - } - const nextRoot = sys.path.dirname(root); - if (nextRoot === root) { - break; - } - root = nextRoot; - } - return undefined; -} - -const findQwikRoots = async ( - sys: OptimizerSystem, - packageJsonDir: string -): Promise => { - const paths = new Map(); - if (sys.env === 'node' || sys.env === 'bun') { - const fs: typeof import('fs') = await sys.dynamicImport('node:fs'); - let prevPackageJsonDir: string | undefined; - do { - try { - const data = await fs.promises.readFile(sys.path.join(packageJsonDir, 'package.json'), { - encoding: 'utf-8', - }); - - try { - const packageJson = JSON.parse(data); - const dependencies = packageJson['dependencies']; - const devDependencies = packageJson['devDependencies']; - - const packages: string[] = []; - if (typeof dependencies === 'object') { - packages.push(...Object.keys(dependencies)); - } - if (typeof devDependencies === 'object') { - packages.push(...Object.keys(devDependencies)); - } - - const basedir = sys.cwd(); - await Promise.all( - packages.map(async (id) => { - const pkgJsonPath = await findDepPkgJsonPath(sys, id, basedir); - if (pkgJsonPath) { - const pkgJsonContent = await fs.promises.readFile(pkgJsonPath, 'utf-8'); - const pkgJson = JSON.parse(pkgJsonContent); - const qwikPath = pkgJson['qwik']; - if (!qwikPath) { - return; - } - // Support multiple paths - const allPaths = Array.isArray(qwikPath) ? qwikPath : [qwikPath]; - for (const p of allPaths) { - paths.set( - await fs.promises.realpath(sys.path.resolve(sys.path.dirname(pkgJsonPath), p)), - id - ); - } - } - }) - ); - } catch (e) { - console.error(e); - } - } catch { - // ignore errors if package.json not found - } - prevPackageJsonDir = packageJsonDir; - packageJsonDir = sys.path.dirname(packageJsonDir); - } while (packageJsonDir !== prevPackageJsonDir); - } - return Array.from(paths).map(([path, id]) => ({ path, id })); -}; - export const isNotNullable = (v: T): v is NonNullable => { return v != null; }; diff --git a/starters/dev-server.ts b/starters/dev-server.ts index eb214b9547b..27be97ac716 100644 --- a/starters/dev-server.ts +++ b/starters/dev-server.ts @@ -43,10 +43,6 @@ const appNames = readdirSync(startersAppsDir).filter( (p) => statSync(join(startersAppsDir, p)).isDirectory() && p !== "base", ); -const rootDir = resolve(__dirname, ".."); -const packagesDir = resolve(rootDir, "packages"); -const qwikRouterMjs = join(packagesDir, "qwik-router", "lib", "index.qwik.mjs"); - /** Used when qwik-router server is enabled */ const qwikRouterVirtualEntry = "@router-ssr-entry"; const entrySsrFileName = "entry.ssr.tsx"; @@ -198,16 +194,7 @@ export { plugins: [ ...plugins, optimizer.qwikVite({ - /** - * normally qwik finds qwik-router via package.json but we don't want that - * because it causes it to try to lookup the special qwik router imports - * even when we're not actually importing qwik-router - */ - disableVendorScan: true, - vendorRoots: enableRouterServer ? [qwikRouterMjs] : [], - entryStrategy: { - type: "segment", - }, + entryStrategy: { type: "segment" }, client: { manifestOutput(manifest) { clientManifest = manifest; From 5a8e8029be326d8d39104693ff355c4eb0b7cadc Mon Sep 17 00:00:00 2001 From: gioboa Date: Mon, 4 Aug 2025 15:57:16 +0200 Subject: [PATCH 3/4] =?UTF-8?q?fix:=20unit=20tests=20=F0=9F=90=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/qwik/src/optimizer/src/plugins/vite.unit.ts | 2 -- starters/e2e/e2e.use-id.e2e.ts | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/qwik/src/optimizer/src/plugins/vite.unit.ts b/packages/qwik/src/optimizer/src/plugins/vite.unit.ts index 7526318d77e..da0a5df2a85 100644 --- a/packages/qwik/src/optimizer/src/plugins/vite.unit.ts +++ b/packages/qwik/src/optimizer/src/plugins/vite.unit.ts @@ -42,7 +42,6 @@ const noExternal = [ '@qwik.dev/core/internal', '@qwik.dev/core/server', '@qwik.dev/core/build', - '@qwik.dev/router', ]; const excludeDeps = [ @@ -53,7 +52,6 @@ const excludeDeps = [ '@qwik.dev/core/jsx-dev-runtime', '@qwik.dev/core/build', '@qwik-client-manifest', - '@qwik.dev/router', '@builder.io/qwik', '@builder.io/qwik-city', ]; diff --git a/starters/e2e/e2e.use-id.e2e.ts b/starters/e2e/e2e.use-id.e2e.ts index d98e732b749..70a8ceedf3d 100644 --- a/starters/e2e/e2e.use-id.e2e.ts +++ b/starters/e2e/e2e.use-id.e2e.ts @@ -1,4 +1,4 @@ -import { test, expect, Locator } from "@playwright/test"; +import { test, expect } from "@playwright/test"; test.describe("use-id", () => { test.beforeEach(async ({ page }) => { From 0cdbd6d252f6693e6246e900976245d1e51f4972 Mon Sep 17 00:00:00 2001 From: gioboa Date: Tue, 5 Aug 2025 10:28:58 +0200 Subject: [PATCH 4/4] =?UTF-8?q?fix:=20fix=20up=20docs=20=F0=9F=90=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/docs/src/repl/worker/repl-dependencies.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/docs/src/repl/worker/repl-dependencies.ts b/packages/docs/src/repl/worker/repl-dependencies.ts index 1c42daddb3d..68e9ee34374 100644 --- a/packages/docs/src/repl/worker/repl-dependencies.ts +++ b/packages/docs/src/repl/worker/repl-dependencies.ts @@ -77,6 +77,8 @@ const _loadDependencies = async (replOptions: ReplInputOptions) => { } if (!isSameQwikVersion(self.qwikCore?.version)) { + // Special case: we allow importing qwik again in the same process. + delete (globalThis as any).__qwik; await exec(QWIK_PKG_NAME, '/core.cjs'); if (self.qwikCore) { console.debug(`Loaded ${QWIK_PKG_NAME}: ${self.qwikCore.version}`);