Skip to content

chore: throw error when importing core twice #7784

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

Open
wants to merge 4 commits into
base: build/v2
Choose a base branch
from
Open
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/lemon-dingos-dance.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 2 additions & 0 deletions packages/docs/src/repl/worker/repl-dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down
1 change: 1 addition & 0 deletions packages/docs/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ export default defineConfig(async () => {
'algoliasearch',
'@algolia/autocomplete-core/dist/esm/reshape',
'algoliasearch/dist/algoliasearch-lite.esm.browser',
'@qwik-ui/headless',
],
},

Expand Down
6 changes: 6 additions & 0 deletions packages/qwik-router/src/ssg/worker-thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Promise<any>>();

Expand Down Expand Up @@ -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<Promise<any>>();

Expand Down
11 changes: 11 additions & 0 deletions packages/qwik/src/core/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
//////////////////////////////////////////////////////////////////////////////////////////
// Protect against duplicate imports
//////////////////////////////////////////////////////////////////////////////////////////
import { version } from './version';
if ((globalThis as any).__qwik && !(globalThis as any).__qwik_testing) {
throw new Error(
`Qwik version ${(globalThis as any).__qwik} already imported while importing ${version}. Verify external vs bundled imports etc.`
);
}
(globalThis as any).__qwik = version;

//////////////////////////////////////////////////////////////////////////////////////////
// Developer Core API
//////////////////////////////////////////////////////////////////////////////////////////
Expand Down
103 changes: 2 additions & 101 deletions packages/qwik/src/optimizer/src/plugins/vite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type {
GlobalInjections,
Optimizer,
OptimizerOptions,
OptimizerSystem,
QwikManifest,
TransformModule,
} from '../types';
Expand All @@ -25,7 +24,6 @@ import {
type NormalizedQwikPluginOptions,
type QwikBuildMode,
type QwikBuildTarget,
type QwikPackages,
type QwikPluginOptions,
} from './plugin';
import { createRollupError, normalizeRollupOutputOptions } from './rollup';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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';
Expand All @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -714,90 +699,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<QwikPackages[]> => {
const paths = new Map<string, string>();
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 = <T>(v: T): v is NonNullable<T> => {
return v != null;
};
Expand Down
2 changes: 0 additions & 2 deletions packages/qwik/src/optimizer/src/plugins/vite.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ const noExternal = [
'@qwik.dev/core/internal',
'@qwik.dev/core/server',
'@qwik.dev/core/build',
'@qwik.dev/router',
];

const excludeDeps = [
Expand All @@ -61,7 +60,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',
];
Expand Down
16 changes: 2 additions & 14 deletions starters/dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -340,6 +327,7 @@ function favicon(_: Request, res: Response) {
}

async function main() {
(globalThis as any).__qwik_testing = true;
const partytownPath = resolve(
startersDir,
"..",
Expand Down
2 changes: 1 addition & 1 deletion starters/e2e/e2e.use-id.e2e.ts
Original file line number Diff line number Diff line change
@@ -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 }) => {
Expand Down
Loading