chore: add ESLint guard against direct third-party imports bypassing bundle#1189
Conversation
|
Thank you for your contribution! I see you hardcoded the list of bundled packages, I am thinking if we could get that list dynamically. We generate build/src/third_party/bundled-packages.json during bundling, but that we cannot rely on in eslint. We could maybe have a test to check if they are in sync instead. Could eslint parse src/third_party/*.ts for imports there to populate that list? Not sure if that could work. In both cases, we would need some special case handling around puppeteer anyways. If either of those is impractical, certainly having hardcoded list is still better than nothing. |
|
Replaced hardcoded list with |
…ndle 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 ChromeDevTools#1123 where direct imports work in development (devDependencies present) but fail in the published npm package (only bundled code available). Closes ChromeDevTools#1123
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.
OrKoN
force-pushed
the
osc/1123-bundle-import-test-guard
branch
from
a9238e5 to
f9e15d3
Compare
OrKoN
changed the title
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
|
Thanks for the merge! |
|
Thanks for the quick review and merge! |
4 participants
Summary
@local/no-direct-third-party-importsthat flags value imports of bundled third-party packages (@modelcontextprotocol/sdk,puppeteer-core,@puppeteer/browsers,yargs,debug,zod,core-js) when used outside ofsrc/third_party/import type) are allowed since they are erased at compile time and don't affect the bundlesrc/**/*.tsso development scripts and tests are unaffectedThis prevents the class of bugs where a direct npm import works during development (devDependencies installed) but breaks in the published package (only bundled code ships). PR #1111 was an example of this exact issue caught through manual
npm packtesting.Closes #1123
Test plan
npx eslint --no-cache src/passes with no violations on the current codebaseimport {Client} from '@modelcontextprotocol/sdk/client/index.js'import type {Flags} from 'lighthouse'(type-only import)src/third_party/index.ts(the barrel itself)src/**/*.ts)