Skip to content

fix: bundle react-scanner and typescript so consumers with typescript 7 work - #803

Merged
jennifer-takagi merged 1 commit into
mainfrom
ktlo/bundle-scanner-deps
Oct 1, 2026
Merged

jennifer-takagi merged 1 commit into
mainfrom
ktlo/bundle-scanner-deps

Conversation

@jennifer-takagi

Copy link
Copy Markdown
Collaborator

Description of change

Follows #802. That PR fixed the local build; this one fixes the published package.

What is wrong. Any consumer whose project has typescript@7 installed crashes the moment they import us:

Cannot read properties of undefined (reading 'BarBarToken')

scanOrg loads react-scanner, which loads @typescript-eslint/typescript-estree, which calls require("typescript") and gets whatever npm hoisted to the consumer's root. TypeScript 7 ships only a launcher for the native Go compiler; its "." export is lib/version.cjs, which has no SyntaxKind.

react-scanner pins typescript@5.6.2 and npm nests it correctly, but estree hoists away from it and resolves the consumer's copy instead. Reproduced in a scratch project:

require.resolve('typescript', {paths: [...consumer.../typescript-estree]})
→ consumer/node_modules/typescript/lib/version.cjs   # the TS7 launcher

The same project without typescript@7 imports fine, so this is squarely about what the consumer happens to have installed. #802 pinned our own typescript to 6.x, but devDependencies never reach consumers, so the published package stayed broken.

There is no upstream fix to wait for. estree 8.71.0, the latest, still declares typescript ">=4.8.4 <6.1.0", and react-scanner has not moved off 1.2.0.

What this changes.

  • bundleDependencies: ["react-scanner", "typescript"] — ships our own copies inside the tarball, so the consumer's typescript stops taking part in resolution. This is the only option that makes the package work rather than fail more clearly.
  • typescript moves to dependencies, pinned to 5.6.2 — it is a real runtime dependency through estree, and the exact pin matches react-scanner's so the two dedupe into one bundled copy instead of two.
  • Dependabot ignores typescript outright, rather than majors only. Any bump, including a patch, splits it from react-scanner's pin and doubles the tarball.

What this costs. The published package goes from 13 kB unpacked across 15 files to 25 MB across 775 files (4.7 MB packed). That is the price of not depending on the consumer's node_modules layout, and it is worth naming out loud.

What I rejected. A peerDependencies range on typescript only moves the failure to install time, where --legacy-peer-deps walks straight back into the same crash. npm overrides do not apply to consumers at all. Pinning estree to a version that forces npm to nest it works by accident of the hoisting algorithm and reads as a mistake to the next maintainer.

How to review. The claim worth a second opinion is the size trade: 25 MB unpacked to support consumers on TypeScript 7. The alternative is telling them to stay on TypeScript 6.

To check it yourself:

npm pack --pack-destination /tmp
mkdir /tmp/c && cd /tmp/c && npm init -y && npm pkg set type=module
npm i typescript@7.0.2 /tmp/jimdo-components-stats-3.2.1.tgz
node -e "import('@jimdo/components-stats').then(m => console.log(Object.keys(m)))"

On main that throws BarBarToken; here it prints [ 'scanOrg' ]. I also ran react-scanner through the bundled copy against a .tsx fixture containing true || false — the exact expression that triggers the crash — and it scanned and reported correctly.

Pull-Request Checklist

  • Code is up-to-date with the main branch
  • npm run lint passes with this change — the repo's script is npm run check (biome); clean on this diff
  • npm run test passes with this change — 3 passed
  • This pull request links relevant issues as Fixes #0000 — N/A, no issue filed
  • There are new or updated unit tests validating the change — N/A, see below
  • Documentation has been updated to reflect this change — N/A
  • The new commits follow conventions outlined in the conventional commit spec

Gap left open. No test imports react-scanner, which is why a bump that killed the package at runtime passed CI twice. The test that would actually catch this is a packaging test: pack, install into a scratch project, import. That is slower than the current suite and belongs in its own PR.

🤖 Generated with Claude Code

… 7 work

Any consumer whose project has typescript@7 installed crashes on import
with "Cannot read properties of undefined (reading 'BarBarToken')".

react-scanner loads @typescript-eslint/typescript-estree, which calls
require("typescript") and gets whatever npm hoisted to the consumer's
root. typescript@7 ships only a launcher for the native Go compiler; its
"." export is lib/version.cjs, which has no SyntaxKind. Upstream has no
fix: estree 8.71.0 still peers typescript <6.1.0, and react-scanner is
unmaintained at 1.2.0.

Declaring a peerDependency would only move the failure to install time,
where --legacy-peer-deps walks straight back into it. Bundling react-
scanner and typescript ships our own copies inside the tarball, so the
consumer's typescript no longer takes part in resolution.

typescript moves from devDependencies to dependencies, pinned to 5.6.2 to
match react-scanner's pin so the two dedupe into one bundled copy. It is
a real runtime dependency through estree. Dependabot now ignores it
entirely; a bump splits the copies and doubles the tarball.

Published size goes from 13 kB unpacked to 25 MB, 15 files to 775.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jennifer-takagi
jennifer-takagi marked this pull request as ready for review October 1, 2026 11:50
@jennifer-takagi
jennifer-takagi requested a review from a team as a code owner October 1, 2026 11:50
@jennifer-takagi
jennifer-takagi merged commit 3013f3c into main Oct 1, 2026
1 check passed
@jennifer-takagi
jennifer-takagi deleted the ktlo/bundle-scanner-deps branch October 1, 2026 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant