From 165f20834e17d743b5c6f06c0b7f8ae6e2bcab9a Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sun, 15 Mar 2026 09:32:02 -0700 Subject: [PATCH 1/3] fix: add ESLint guard against direct third-party imports bypassing bundle Adds a custom ESLint rule `@local/no-direct-third-party-imports` that prevents value imports of bundled third-party packages from files in `src/` unless they go through the `src/third_party/index.ts` barrel. Type-only imports are allowed since they are erased at compile time. The rule is scoped to `src/**/*.ts` so scripts and tests are unaffected. This prevents the class of bugs described in #1123 where direct imports work in development (devDependencies present) but fail in the published npm package (only bundled code available). Closes #1123 --- eslint.config.mjs | 7 ++ scripts/eslint_rules/local-plugin.js | 8 +- .../no-direct-third-party-imports-rule.js | 95 +++++++++++++++++++ 3 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 scripts/eslint_rules/no-direct-third-party-imports-rule.js diff --git a/eslint.config.mjs b/eslint.config.mjs index b1a6121c6..d90b80e9e 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -136,6 +136,13 @@ export default defineConfig([ }, }, { + name: 'Bundle import guard', + files: ['src/**/*.ts'], + rules: { + '@local/no-direct-third-party-imports': 'error', + }, + }, + { name: 'Tests', files: ['**/*.test.ts'], rules: { diff --git a/scripts/eslint_rules/local-plugin.js b/scripts/eslint_rules/local-plugin.js index 27a20d372..d8b3168e3 100644 --- a/scripts/eslint_rules/local-plugin.js +++ b/scripts/eslint_rules/local-plugin.js @@ -5,5 +5,11 @@ */ import checkLicenseRule from './check-license-rule.js'; +import noDirectThirdPartyImportsRule from './no-direct-third-party-imports-rule.js'; -export default {rules: {'check-license': checkLicenseRule}}; +export default { + rules: { + 'check-license': checkLicenseRule, + 'no-direct-third-party-imports': noDirectThirdPartyImportsRule, + }, +}; diff --git a/scripts/eslint_rules/no-direct-third-party-imports-rule.js b/scripts/eslint_rules/no-direct-third-party-imports-rule.js new file mode 100644 index 000000000..3a809beb1 --- /dev/null +++ b/scripts/eslint_rules/no-direct-third-party-imports-rule.js @@ -0,0 +1,95 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * ESLint rule that prevents value (non-type) imports of third-party packages + * that should go through the `src/third_party/index.ts` barrel file. + * + * Type-only imports are allowed because they are erased at compile time and + * do not affect the bundle. + * + * This catches a class of bugs where a direct import works in development + * (because devDependencies are installed) but fails once the package is + * bundled and published via `npm pack`. + * + * See https://github.com/ChromeDevTools/chrome-devtools-mcp/issues/1123 + */ + +const THIRD_PARTY_PACKAGES = [ + '@modelcontextprotocol/sdk', + 'puppeteer-core', + '@puppeteer/browsers', + 'yargs', + 'debug', + 'zod', + 'core-js', +]; + +/** Matches any import source that starts with one of the restricted packages. */ +function isRestrictedSource(source) { + return THIRD_PARTY_PACKAGES.some( + pkg => source === pkg || source.startsWith(pkg + '/'), + ); +} + +/** Returns true when the file is inside src/third_party/. */ +function isThirdPartyBarrel(filename) { + const normalized = filename.replace(/\\/g, '/'); + return normalized.includes('/src/third_party/'); +} + +export default { + name: 'no-direct-third-party-imports', + meta: { + type: 'problem', + docs: { + description: + 'Disallow value imports of bundled third-party packages outside of src/third_party/', + }, + schema: [], + messages: { + noDirectImport: + 'Do not import "{{source}}" directly. Use the re-export from "src/third_party/index.js" instead so the import survives bundling. (Type-only imports are fine.)', + }, + }, + defaultOptions: [], + create(context) { + const filename = context.getFilename(); + if (isThirdPartyBarrel(filename)) { + return {}; + } + + return { + ImportDeclaration(node) { + // `import type { Foo } from '...'` is always safe. + if (node.importKind === 'type') { + return; + } + + const source = node.source.value; + if (!isRestrictedSource(source)) { + return; + } + + // If every specifier is `type`, the import is still safe. + // e.g. `import { type Foo, type Bar } from '...'` + const hasValueSpecifier = node.specifiers.some( + s => s.type !== 'ImportSpecifier' || s.importKind !== 'type', + ); + + if (!hasValueSpecifier) { + return; + } + + context.report({ + node, + messageId: 'noDirectImport', + data: {source}, + }); + }, + }; + }, +}; From f9e15d3c57e68b7ace9a04b882b63dce8fd8f5ec Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Thu, 19 Mar 2026 11:16:29 -0700 Subject: [PATCH 2/3] Dynamically populate bundled packages from src/third_party imports Replace the hardcoded THIRD_PARTY_PACKAGES list with dynamic parsing of src/third_party/*.ts files at ESLint load time. This extracts bare package names from import/export statements and side-effect imports, so the list stays in sync automatically as new packages are added. A sync test against build/src/third_party/bundled-packages.json can be added once that file is generated during the build step. --- .../no-direct-third-party-imports-rule.js | 61 ++++++++++++++++--- 1 file changed, 52 insertions(+), 9 deletions(-) diff --git a/scripts/eslint_rules/no-direct-third-party-imports-rule.js b/scripts/eslint_rules/no-direct-third-party-imports-rule.js index 3a809beb1..a58fef2c5 100644 --- a/scripts/eslint_rules/no-direct-third-party-imports-rule.js +++ b/scripts/eslint_rules/no-direct-third-party-imports-rule.js @@ -15,18 +15,61 @@ * (because devDependencies are installed) but fails once the package is * bundled and published via `npm pack`. * + * The list of bundled packages is derived dynamically by scanning + * `src/third_party/*.ts` for import/export statements at ESLint load time. + * * See https://github.com/ChromeDevTools/chrome-devtools-mcp/issues/1123 */ -const THIRD_PARTY_PACKAGES = [ - '@modelcontextprotocol/sdk', - 'puppeteer-core', - '@puppeteer/browsers', - 'yargs', - 'debug', - 'zod', - 'core-js', -]; +import {readdirSync, readFileSync} from 'node:fs'; +import {join, dirname} from 'node:path'; +import {fileURLToPath} from 'node:url'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const THIRD_PARTY_DIR = join(__dirname, '..', '..', 'src', 'third_party'); + +/** + * Parse all .ts files in src/third_party/ and extract the bare package names + * from import/export statements. Relative imports and node_modules paths + * (used for chrome-devtools-frontend) are skipped. + */ +function discoverBundledPackages() { + const packages = new Set(); + // Match `from 'pkg'` (may appear on a different line than `import`) + // and side-effect imports like `import 'pkg'`. + const fromRe = /from\s+['"]([^'"]+)['"]/g; + const sideEffectRe = /^import\s+['"]([^'"]+)['"]/gm; + + let files; + try { + files = readdirSync(THIRD_PARTY_DIR).filter(f => f.endsWith('.ts')); + } catch { + return []; + } + + for (const file of files) { + const content = readFileSync(join(THIRD_PARTY_DIR, file), 'utf8'); + for (const re of [fromRe, sideEffectRe]) { + re.lastIndex = 0; + let match; + while ((match = re.exec(content)) !== null) { + const source = match[1]; + // Skip relative imports and node_modules paths. + if (source.startsWith('.') || source.startsWith('/')) { + continue; + } + // Extract the bare package name (handle scoped packages like @foo/bar). + const parts = source.split('/'); + const pkg = source.startsWith('@') ? parts.slice(0, 2).join('/') : parts[0]; + packages.add(pkg); + } + } + } + + return [...packages]; +} + +const THIRD_PARTY_PACKAGES = discoverBundledPackages(); /** Matches any import source that starts with one of the restricted packages. */ function isRestrictedSource(source) { From d747b371e5d119d55f6f684719ccb25d04387a17 Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Wed, 1 Apr 2026 09:07:18 +0200 Subject: [PATCH 3/3] chore: review comments --- eslint.config.mjs | 4 ++-- .../no-direct-third-party-imports-rule.js | 18 ++++++++++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index d90b80e9e..33d2a1d63 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -136,13 +136,13 @@ export default defineConfig([ }, }, { - name: 'Bundle import guard', + name: 'Source files', files: ['src/**/*.ts'], rules: { '@local/no-direct-third-party-imports': 'error', }, }, - { + { name: 'Tests', files: ['**/*.test.ts'], rules: { diff --git a/scripts/eslint_rules/no-direct-third-party-imports-rule.js b/scripts/eslint_rules/no-direct-third-party-imports-rule.js index a58fef2c5..7514418fd 100644 --- a/scripts/eslint_rules/no-direct-third-party-imports-rule.js +++ b/scripts/eslint_rules/no-direct-third-party-imports-rule.js @@ -22,11 +22,15 @@ */ import {readdirSync, readFileSync} from 'node:fs'; -import {join, dirname} from 'node:path'; -import {fileURLToPath} from 'node:url'; +import {join} from 'node:path'; -const __dirname = dirname(fileURLToPath(import.meta.url)); -const THIRD_PARTY_DIR = join(__dirname, '..', '..', 'src', 'third_party'); +const THIRD_PARTY_DIR = join( + import.meta.dirname, + '..', + '..', + 'src', + 'third_party', +); /** * Parse all .ts files in src/third_party/ and extract the bare package names @@ -60,7 +64,9 @@ function discoverBundledPackages() { } // Extract the bare package name (handle scoped packages like @foo/bar). const parts = source.split('/'); - const pkg = source.startsWith('@') ? parts.slice(0, 2).join('/') : parts[0]; + const pkg = source.startsWith('@') + ? parts.slice(0, 2).join('/') + : parts[0]; packages.add(pkg); } } @@ -100,7 +106,7 @@ export default { }, defaultOptions: [], create(context) { - const filename = context.getFilename(); + const filename = context.filename; if (isThirdPartyBarrel(filename)) { return {}; }