fix(ts): anchor.BN undefined in ESM — guard CJS globals in index.ts#4525
fix(ts): anchor.BN undefined in ESM — guard CJS globals in index.ts#4525eteen12 wants to merge 3 commits into
Conversation
|
@eteen12 is attempting to deploy a commit to the Solana Foundation Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR fixes a
Confidence Score: 4/5Safe to merge for the targeted fix; the two minor package.json concerns (no The ts/packages/anchor/package.json — the new Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Consumer["Consumer imports\n@anchor-lang/core"]
Consumer -->|"exports field\npresent (new)"| ExportsMap["exports map resolution"]
ExportsMap -->|"browser condition\n(bundler, browser target)"| BrowserBundle["dist/browser/index.js\n(browser bundle)"]
ExportsMap -->|"import condition\n(Node.js ESM / SSR bundler)"| ESMBundle["dist/esm/index.js\n(ESM build)"]
ExportsMap -->|"require condition\n(Node.js CJS)"| CJSBundle["dist/cjs/index.js\n(CJS build)"]
ESMBundle --> Guard{"typeof exports\n!== 'undefined'?\n(new guard)"}
Guard -->|"false — ESM\nno ReferenceError"| ExportsOK["Module loads cleanly\nBN, web3, etc. available"]
Guard -->|"true — CJS"| CJSBlock["exports.workspace = ...\nexports.Wallet = ..."]
CJSBundle --> CJSBlock2["exports.workspace = ...\nexports.Wallet = ..."]
CJSBlock2 --> ExportsOK2["Module loads cleanly\nworkspace & Wallet available"]
Reviews (1): Last reviewed commit: "fix(ts): guard CJS globals in index.ts s..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Fixes initialization failure of the ESM entrypoint (used by modern bundlers) and formalizes package entrypoint routing via package.json#exports so import/require/browser consumers resolve to the intended builds.
Changes:
- Guard the CJS-only
exports/requireblock insrc/index.tsso ESM consumers don’t crash on module init. - Add an
"exports"map inpackage.jsonto routebrowser/import/requireto the correct dist outputs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ts/packages/anchor/src/index.ts | Adds a runtime guard to prevent CJS globals usage from crashing the ESM build at import time. |
| ts/packages/anchor/package.json | Introduces an "exports" map to define explicit entrypoints for browser, ESM importers, and CJS require. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
fixed bot feedback |
1d39c15 to
6fb73ae
Compare
fixes otter-sec#3711 dist/esm/index.js is what bundlers (Vite, Webpack 5, esbuild) load via the "module" field. that file gets these lines verbatim from TypeScript: if (!isBrowser) { exports.workspace = require("./workspace.js").default; exports.Wallet = require("./nodewallet.js").default; } exports and require are CJS globals — they don't exist in ESM, so the whole module throws a ReferenceError on init and anchor.BN (along with everything else) comes back undefined. users see it as "anchor.BN is not a constructor" with no obvious reason why. wrap the CJS block in typeof exports !== "undefined" so the ESM build skips it instead of crashing.
6fb73ae to
541b551
Compare
Co-authored-by: Jamie Hill-Daniel <134328753+jamie-osec@users.noreply.github.com>
Co-authored-by: Jamie Hill-Daniel <134328753+jamie-osec@users.noreply.github.com>
fixes #3711
what's happening
dist/esm/index.jsis what bundlers (Vite, Webpack 5, esbuild) load when they follow the"module"field. that file gets these lines verbatim from TypeScript:exportsandrequireare CJS globals — they don't exist in ESM. so the whole module throws aReferenceErroron init andanchor.BN(along with everything else) comes backundefined. users see it asanchor.BN is not a constructorwith no obvious reason why.changes
src/index.ts— wrap the CJS block intypeof exports !== "undefined"so the ESM build skips it instead of crashingwhy no
exportsfieldan earlier revision of this PR also added an
"exports"field topackage.jsonto make the import/require/browser routing explicit. dropped it: onceexportsis defined, Node disables the legacy.jsand/index.jsauto-resolution for subpaths, which broke a handful of in-repo tests (and would silently break any downstream user) doing deep imports like@anchor-lang/core/dist/cjs/.... the source guard alone fixes the original BN crash; bundlers still pick up"module", Node still uses"main", nothing else needs to change.