Repository navigation
fix: bundle react-scanner and typescript so consumers with typescript 7 work - #803
Merged
Merged
Conversation
… 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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@7installed crashes the moment they import us:scanOrgloadsreact-scanner, which loads@typescript-eslint/typescript-estree, which callsrequire("typescript")and gets whatever npm hoisted to the consumer's root. TypeScript 7 ships only a launcher for the native Go compiler; its"."export islib/version.cjs, which has noSyntaxKind.react-scanner pins
typescript@5.6.2and npm nests it correctly, but estree hoists away from it and resolves the consumer's copy instead. Reproduced in a scratch project:The same project without
typescript@7imports fine, so this is squarely about what the consumer happens to have installed. #802 pinned our owntypescriptto 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'stypescriptstops taking part in resolution. This is the only option that makes the package work rather than fail more clearly.typescriptmoves todependencies, pinned to5.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.typescriptoutright, 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
peerDependenciesrange ontypescriptonly moves the failure to install time, where--legacy-peer-depswalks straight back into the same crash. npmoverridesdo 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:
On
mainthat throwsBarBarToken; here it prints[ 'scanOrg' ]. I also ran react-scanner through the bundled copy against a.tsxfixture containingtrue || false— the exact expression that triggers the crash — and it scanned and reported correctly.Pull-Request Checklist
mainbranchnpm run lintpasses with this change — the repo's script isnpm run check(biome); clean on this diffnpm run testpasses with this change — 3 passedFixes #0000— N/A, no issue filedGap 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