chore: add ES module support for browser compatibility #137
Conversation
WalkthroughAdds an ES module build and packaging surface: ignore Changes
Sequence Diagram(s)sequenceDiagram
participant App as Consumer
participant Resolver as Platform resolver
participant Package as @trufnetwork/sdk-js
note over Package: package.json exposes "exports" and "module"
App->>Resolver: import { X } from '@trufnetwork/sdk-js'
alt ESM import resolution
Resolver->>Package: resolve "import"
Note right of Package #DFF7DF: returns ./dist-esm/index.js (ESM)
Resolver-->>App: ./dist-esm/index.js
else CommonJS require
Resolver->>Package: resolve "require"
Note right of Package #F0F0F0: returns ./dist/index.js (CJS)
Resolver-->>App: ./dist/index.js
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Time Submission Status
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
tsconfig.esm.json (1)
4-7: Prefer Node16/NodeNext resolution for ESM-aware buildsWith TypeScript 4.9,
moduleResolution: "node16"(or"nodenext") improves resolution for packages withexports. This can reduce edge-case resolution issues when targeting ESM.Apply this diff:
- "module": "ES2020", - "target": "ES2020", - "moduleResolution": "node", + "module": "ES2020", + "target": "ES2020", + "moduleResolution": "node16",package.json (2)
31-46: Pack flow doesn’t build/publish ESM outputs
packusespack_buildwhich only buildsdist. If you rely onpnpm run packfor release validation, it should also builddist-esm(or reusebuild).Apply this diff:
- "pack_build": "pnpm run clean && tsc --outDir ./dist --sourceMap false --declarationDir ./dist", + "pack_build": "pnpm run build",
1-98: Optional: help bundlers and tree‑shakingConsider adding:
"sideEffects": falseif the package has no top-level side effects.- A
defaultcondition inexportsfor older tooling fallbacks.Example:
"exports": { ".": { "import": "./dist-esm/index.js", "require": "./dist/index.js", "types": "./dist/index.d.ts" } }, + "sideEffects": false,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)package.json(1 hunks)tsconfig.esm.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (3)
.gitignore (1)
11-11: LGTM: ignore dist-esm artifactsAdding
/dist-esmmirrors the existing/distrule and keeps build outputs out of VCS. No issues.package.json (2)
8-14: Export map looks good — confirm no historical deep importsOnly "." is exported in package.json (exports section); that will break consumers that deep-import internals (e.g., @trufnetwork/kwil-js/dist/...). I ran the suggested ripgrep scan in this repo and it returned no matches. Absence of matches is not conclusive — verify downstream consumers, examples/docs/CI and any published packages for deep imports. Command to run locally: rg -n "@trufnetwork/kwil-js/" -S || true
76-83: Sanity-check CJS build for ESM-only dep (ethers v6)package.json publishes a CJS entry (main: ./dist/index.js, exports.require -> ./dist/index.js) but there is no dist/ build in this branch so I couldn't inspect the compiled CJS. Build the package and search the CJS output for require('ethers'); if any require('ethers') exist, switch those to dynamic import() or ship ESM-only to avoid runtime errors with ethers v6.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
tsconfig.esm.json (1)
8-11: ESM config looks good; browser libs and no maps.You added DOM libs and disabled maps; aligns with browser ESM output. This addresses the prior comment about adding browser libs and removing invalid declarationDir.
package.json (1)
6-14: Exports map for dual CJS/ESM LGTM.Top-level “module” and conditional “exports” for import/require/types are set correctly for bundlers and Node.
🧹 Nitpick comments (8)
tsconfig.esm.json (2)
3-11: Prefer bundler-friendly resolution and stable ESM semantics.For browser toolchains, use moduleResolution "bundler" and enable verbatimModuleSyntax to avoid TS inserting CJS interop and to align with ESM packages.
Apply:
"compilerOptions": { - "module": "ES2020", - "target": "ES2020", - "moduleResolution": "node", + "module": "ES2020", + "target": "ES2020", + "moduleResolution": "bundler", "outDir": "./dist-esm", "lib": ["DOM", "ES2020"], "declaration": true, + "verbatimModuleSyntax": true, "declarationMap": false, "sourceMap": false }
8-11: Avoid duplicate .d.ts outputs or repoint “types”.You emit declarations for both CJS (dist) and ESM (dist-esm) but the package “types” points to dist/index.d.ts. Either disable ESM declarations or repoint “types” to dist-esm. Pick one to reduce confusion/size.
Option A (disable ESM .d.ts):
- "declaration": true, + "declaration": false,Option B (keep ESM .d.ts): update package.json “types” and exports.types to dist-esm (see package.json comment).
package.json (6)
19-29: Avoid publishing unnecessary sources and duplicated .d.ts.Consider dropping dist//*.ts and dist-esm//.d.ts (if “types” points to dist/) to keep the tarball lean.
"files": [ "dist/**/*.js", - "dist/**/*.ts", "dist-esm/**/*.js", - "dist-esm/**/*.d.ts", "dist-esm/package.json",If you keep ESM declarations (tsconfig.esm.json), also repoint “types”/exports.types to dist-esm instead (see below).
33-36: Make postbuild cross‑platform and idempotent.echo with quotes can break on Windows shells. Use Node to write dist-esm/package.json.
- "build": "pnpm run clean && tsc && tsc -p tsconfig.esm.json && echo '{\"type\":\"module\"}' > dist-esm/package.json", + "build": "pnpm run clean && tsc && tsc -p tsconfig.esm.json && node -e \"const fs=require('fs');fs.mkdirSync('dist-esm',{recursive:true});fs.writeFileSync('dist-esm/package.json','{\\\"type\\\":\\\"module\\\"}\\n');\"",
6-13: If keeping ESM .d.ts, align the “types” entry.If tsconfig.esm.json continues to emit declarations, point “types” and exports.types at dist-esm to match the ESM import path and reduce mismatches.
- "types": "./dist/index.d.ts", + "types": "./dist-esm/index.d.ts", "exports": { ".": { "import": "./dist-esm/index.js", "require": "./dist/index.js", - "types": "./dist/index.d.ts" + "types": "./dist-esm/index.d.ts" } },
8-14: Expose package.json via exports for tooling.Small DX win for bundlers and analyzers.
"exports": { ".": { "import": "./dist-esm/index.js", "require": "./dist/index.js", "types": "./dist/index.d.ts" } + ,"./package.json": "./package.json" },
33-48: Add an ESM type‑check script and CI guard.Ensure the ESM build type‑checks with DOM libs (tsconfig.esm.json).
"scripts": { "clean": "rimraf dist/* dist-esm/*", "build": "pnpm run clean && tsc && tsc -p tsconfig.esm.json && echo '{\"type\":\"module\"}' > dist-esm/package.json", + "type-check:esm": "tsc -p tsconfig.esm.json --noEmit",Consider running both type-check and type-check:esm in CI.
1-14: Optional: add “sideEffects”: false for better tree‑shaking.If modules are pure on import, mark as side-effect free.
{ "name": "@trufnetwork/kwil-js", "version": "0.9.5", "description": "", "main": "./dist/index.js", "module": "./dist-esm/index.js", + "sideEffects": false,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json(1 hunks)tsconfig.esm.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (3)
package.json (3)
6-14: Optional: add “browser” hint.If any Node shims are used, a browser field can aid bundlers.
"main": "./dist/index.js", "module": "./dist-esm/index.js", + "browser": "./dist-esm/index.js",Verify this doesn’t break Node resolution in your consumers.
6-14: Confirm: TypeScript resolves the "types" condition in package.json "exports" (since v4.7).
TypeScript added support in v4.7 and matches the "types" (and "default") conditions; your top-level "types" fallback covers pre-4.7 consumers — no change required.
78-85: Risk: CJS entry (./dist/index.js) vs ESM-only dependency (ethers v6) — verify buildpackage.json exposes exports.require -> ./dist/index.js and dependencies include "ethers": "^6.9.1" (ethers v6 is ESM‑only). The repo has no dist/ so I could not run the require() runtime check.
Action — reproduce and confirm:
- Build the CJS bundle: pnpm run pack_build
- Run the runtime check:
node -e "try{require('./dist/index.js');console.log('CJS ok')}catch(e){console.error('CJS failed:',e.message);process.exit(1)}"If the check fails because ethers is ESM‑only, choose one: remove/publish only ESM (drop the require export), or make the CJS entry lazy-load ethers with dynamic import('ethers') behind a function boundary, or provide an ESM wrapper instead of a CJS require.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tsconfig.esm.json (3)
3-11: Use NodeNext resolution for ESM correctness.With TS 4.9, pairing
"module": "NodeNext"and"moduleResolution": "NodeNext"aligns type resolution with package "exports"/conditional types and prevents subtle ESM/CJS resolver mismatches.- "target": "ES2021", - "module": "ES2020", + "target": "ES2021", + "module": "NodeNext", ... - "moduleResolution": "node", + "moduleResolution": "NodeNext",
12-12: Add DOM.Iterable for browser iterables.Enables proper typing for Headers/FormData/URLSearchParams iteration in browsers.
- "lib": ["DOM", "ES2020"], + "lib": ["DOM", "DOM.Iterable", "ES2020"],
6-7: Minor: redundant flag.
allowSyntheticDefaultImportsis implied whenesModuleInteropis true; you can drop it.- "esModuleInterop": true, + "esModuleInterop": true, ... - "allowSyntheticDefaultImports": trueAlso applies to: 17-17
package.json (1)
9-16: Exports map looks good; consider broader compatibility.The dual export for import/require + types is correct. Optionally add a
"default"condition as a safe fallback.".": { + "default": "./dist-esm/index.js", "import": "./dist-esm/index.js", "require": "./dist/index.js", "types": "./dist/index.d.ts" },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json(1 hunks)tsconfig.esm.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (5)
tsconfig.esm.json (2)
13-14: Confirm intentional: no .d.ts from ESM build.
declaration: falsemeans consumers get types only from CJS (dist/index.d.ts). Fine if surfaces are identical; otherwise types may drift.Please confirm your base build emits up-to-date
dist/index.d.tsand matches the ESM entry points.
19-27: Broaden/DRY the ESM include (avoid missing ESM artifacts)Extend the base tsconfig and replace the hard-coded include list with a recursive glob so all source files (e.g., BrowserTNClient) are included; repo search returned no matches for BrowserTNClient — cannot verify automatically.
+{ + "extends": "./tsconfig.json", "compilerOptions": { "rootDir": "./src", "outDir": "./dist-esm", ... }, - "include": [ - "src/index.ts", - "src/core/*.ts", - "src/funder/abi/*.ts", - "src/funder/*.ts", - "src/client/kwil.ts", - "src/api_client/config.ts", - "src/utils/*.ts" - ] + "include": ["src/**/*.ts"] }Confirm BrowserTNClient exists in the repo and that tsconfig.esm.json change will include it.
package.json (3)
35-37: LGTM on emitting dist-esm/package.json.Creating
{ "type": "module" }indist-esmavoids Node treating ESM.jsas CJS.
8-8: Verify sideEffects: false won't drop required top-level initializationQuick grep for 'register|init|polyfill' in src/ returned no matches; scan is not exhaustive.
- Confirm package.json entry points (main/module/exports) and entry files (index.js, src/index.ts) do not rely on top-level side-effectful imports (polyfills, shims, registrations).
- If unsure, run a broader repo search: rg -nP '\b(polyfill|register|init|shim|patch|setup)\b' -S -C2.
1-16: Optional: add a smoke-test to catch the original browser import error.Add a CI job that builds the ESM bundle, packs the package, and runs a minimal Vite/Webpack import of BrowserTNClient to prevent regressions like “Class extends value undefined”.
Findings: package.json exports point to ./dist-esm/index.js (import) and ./dist/index.js (require); dist and dist-esm are not present in the repo and no BrowserTNClient symbol was found — unable to run smoke locally; CI should run this against the packaged build.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
42-46: Pack script omits ESM artifacts —pnpm --prefix ./dist packpacks from ./dist and excludes ./dist-esm.This will publish a tarball without the ESM build, undermining the PR goal.
Proposed fix: pack from repo root (uses root "files" list) and reuse the existing
build:- "pack_build": "pnpm run clean && tsc --outDir ./dist --sourceMap false --declarationDir ./dist", - "pack_pre": "copyfiles package.json README.md dist && rimraf ./kwil*.tgz", + "pack_build": "pnpm run build", + "pack_pre": "rimraf ./kwil*.tgz", - "pack": "pnpm run pack_build && pnpm run pack_pre && pnpm --prefix ./dist pack && pnpm run pack_post", + "pack": "pnpm run pack_build && pnpm run pack_pre && pnpm pack && pnpm run pack_post",
♻️ Duplicate comments (1)
tsconfig.esm.json (1)
11-11: Remove invaliddeclarationDir: null(TS config error).
declarationDirmust be a string path. With declarations disabled it should be omitted."declaration": false, - "declarationDir": null, "declarationMap": false,
🧹 Nitpick comments (3)
tsconfig.esm.json (2)
10-14: Clarify ESM declaration strategy.ESM build disables declarations while package.json publishes
dist-esm/**/*.d.ts. Either:
- keep a single type source (CJS) and drop ESM
.d.tsfrom files, or- emit ESM
.d.tstoo (setdeclaration: trueanddeclarationDir: "./dist-esm").Which do you intend?
5-6: Optional: considermoduleResolution: "Bundler"when upgrading to TS ≥5.Helps browser/bundler ESM resolution (optional future tweak). Current NodeNext is fine with TS 4.9.
package.json (1)
21-31: Files list vs emitted artifacts.You include
dist-esm/**/*.d.tsbut ESM declarations are not emitted. Either remove that line or enable ESM declarations (see tsconfig comment).- "dist-esm/**/*.d.ts",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json(1 hunks)tsconfig.esm.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (4)
package.json (4)
9-16: Exports map looks good for dual CJS/ESM.Please confirm no consumers rely on deep imports, since only “.” and “./package.json” are exported.
36-39: LGTM on ESM marker generation.Creating
dist-esm/package.jsonwith{ "type": "module" }is the right call.
8-8: Double-check side-effect safety.
"sideEffects": falseenables tree‑shaking. Verify no modules rely on side effects (polyfills, global patches, CSS-in-JS auto‑injection, etc.).
81-88: Verify CommonJS entry won't require() ESM-only deps (uuid, lru-cache)
- Evidence: code imports 'uuid' (src/utils/uuid.ts) and 'lru-cache' (src/client/kwil.ts); npm metadata shows lru-cache and uuid publish with "type":"module" while ethers v6 still exposes a CJS fallback.
- If you must support CommonJS: ensure exports.require (./dist/index.js) does not require() ESM-only entrypoints — options: dynamic import(), createRequire(), consume a package-provided CJS 'require' export, or publish ESM‑only and drop CJS.
- Verify now by inspecting each dependency's package.json 'exports' for a 'require' target or by running Node to require your built ./dist/index.js in a pure CommonJS environment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tsconfig.esm.json (1)
11-11: Remove invalid compiler option: declarationDir: null.TypeScript expects a string path;
nullcan breaktsceven when declarations are disabled.Apply:
"declaration": false, - "declarationDir": null, "declarationMap": false,
🧹 Nitpick comments (4)
tsconfig.esm.json (2)
5-6: Use NodeNext/Node16 resolution for ESM-aware dependency typing.
moduleResolution: "node"can mis-resolve packages with "exports" (e.g., ethers v6). Prefer"nodenext"(TS 4.9 supports it).- "moduleResolution": "node", + "moduleResolution": "nodenext",
4-4: Align libs with target.Target is ES2021 but libs pin ES2020; bump to keep type surface consistent.
- "target": "ES2021", - ... - "lib": ["DOM", "DOM.Iterable", "ES2020"], + "target": "ES2021", + ... + "lib": ["DOM", "DOM.Iterable", "ES2021"],Also applies to: 9-9
package.json (2)
9-16: Add "browser" export condition.Helps bundlers prefer the ESM build in web targets.
"exports": { ".": { + "browser": "./dist-esm/index.js", "import": "./dist-esm/index.js", "require": "./dist/index.js", "types": "./dist/index.d.ts" }, "./package.json": "./package.json" },
21-27: Prune unused ESM .d.ts glob.ESM build doesn’t emit declarations; keep the publish list lean.
"files": [ "dist/**/*.js", "dist/**/*.d.ts", "dist-esm/**/*.js", - "dist-esm/**/*.d.ts", "dist-esm/package.json",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(1 hunks)src/index.ts(1 hunks)tsconfig.esm.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
src/index.ts (1)
98-98: Good addition: re‑export EnvironmentType at top level.This improves DX for browser users. Confirm it’s included in the public API docs/changelog for 0.9.x.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
package.json (1)
23-26: Types and ESM files correctly included in publishIncluding
dist/**/*.d.tsanddist-esm/**(withdist-esm/package.json) addresses missing typings and ESM markers noted earlier. Looks good.Also applies to: 30-30
🧹 Nitpick comments (2)
package.json (2)
9-16: Enablingexportscan break deep importsConsumers using deep paths (e.g.,
@trufnetwork/kwil-js/some/internal) will now resolve only"."and"./package.json". Consider documenting this as a breaking change, or add explicit subpath exports if you support them.
42-46: Pack from repo root fixed; remove unusedpack_buildscriptPacking via
pnpm packat repo root solves the prior bad tarball issue.pack_buildnow appears unused; drop it to avoid confusion.Apply this minimal clean-up:
"scripts": { - "pack_build": "pnpm run clean && tsc --outDir ./dist --sourceMap false --declarationDir ./dist", "pack_pre": "rimraf ./kwil*.tgz", "pack_post": "copyfiles ./kwil*.tgz ./pkg && rimraf ./kwil*.tgz", "pack": "pnpm run build && pnpm run pack_pre && pnpm pack && pnpm run pack_post", }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(1 hunks)src/index.ts(1 hunks)tsconfig.esm.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/index.ts
- tsconfig.esm.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (4)
package.json (4)
6-6: Keepmodulefor bundlers;exportsnow drives resolutionAdding
"module": "./dist-esm/index.js"is fine for legacy bundlers, but Node will use"exports". No change requested.
36-39: Build flow LGTM; ESM marker generation is correctDual
tscbuilds plus writingdist-esm/package.jsonensures Node treats ESM as such. No change requested.
9-16: CJS export may break — verify ./dist/index.js doesn't require ESM-only 'ethers'package.json depends on ethers (^6.9.1) and many source files import 'ethers'; ./dist/ is missing so the compiled CJS output couldn't be inspected. If ./dist/index.js ends up calling require('ethers') consumers using require() will throw ERR_REQUIRE_ESM.
- Fix options: ship ESM-only (remove exports["."].require), convert CJS build to use dynamic await import() for ESM-only deps, or dual-publish a CJS-compatible alternative/fallback.
- Action: run the build and inspect ./dist/index.js for require('ethers') (or share the compiled CJS output) and update package.json exports accordingly.
8-8: VerifysideEffects: falseis safeEnsure no module relies on import-time side effects (polyfills, global/window/globalThis mutations, prototype patches, or process.env access/assignments); otherwise bundlers may tree‑shake necessary code. My scan couldn’t search files — run the following in the repo root and paste results:
#!/bin/bash set -euo pipefail echo "1) polyfills / runtime imports" rg -n -S -C2 --hidden --glob '!node_modules/**' '(@babel\/polyfill|core-js|zone\.js|regenerator-runtime|polyfill)' . || true echo "2) Object.defineProperty/assign on global/window/globalThis" rg -n -S -C2 --hidden --glob '!node_modules/**' 'Object\.(defineProperty|assign)\s*\(\s*(globalThis|window|global)\b' . || true echo "3) direct global assignments" rg -n -S -C2 --hidden --glob '!node_modules/**' '(globalThis|window|global)\.[A-Za-z_$][A-Za-z0-9_$]*\s*=' . || true echo "4) process.env usage" rg -n -S -C2 --hidden --glob '!node_modules/**' 'process\.env' . || true echo "5) prototype modifications" rg -n -S -C2 --hidden --glob '!node_modules/**' '(Object|Array|String|Date|HTMLElement|Node)\.prototype' . || true echo "6) package.json sideEffects:false" rg -n --hidden --glob '!node_modules/**' '"sideEffects"\s*:\s*false' . || true
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
package.json (2)
21-31: files includes dist‑esm//*.d.ts but ESM build disables declarations**Since tsconfig.esm.json sets "declaration": false, no .d.ts will be emitted under dist-esm. Keeping the glob is harmless but redundant. Either enable ESM declarations or remove the pattern to avoid confusion.
"files": [ "dist/**/*.js", "dist/**/*.d.ts", "dist-esm/**/*.js", - "dist-esm/**/*.d.ts", "dist-esm/package.json",
1-34: Consider publishConfig for scoped package visibilityScoped packages default to restricted on npm. Add publishConfig to avoid accidental private publishes.
{ "name": "@trufnetwork/kwil-js", "version": "0.9.5", + "publishConfig": { "access": "public" }, "description": "",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(1 hunks)src/index.ts(1 hunks)tsconfig.esm.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tsconfig.esm.json
- src/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (4)
package.json (4)
42-45: Packing from repo root — good fix for dual export mapsThis avoids the previous tarball path skew. LGTM.
5-16: Exports map looks correct for dual CJS/ESM; keep "module" for older bundlersThe import/require split with a root types pointer is solid. No changes needed here.
If you support TypeScript <4.7 consumers, consider adding typesVersions for safety. Want a snippet?
81-83: CJS export present; Ethers v6 is ESM‑only — CJS consumers will likely hit ERR_REQUIRE_ESMpackage.json exports map "require" → ./dist/index.js while source has runtime imports of "ethers" (e.g., src/core/signature.ts, src/core/kwilSigner.ts, src/transaction/payloadTx.ts). dist/ is not present in the repo so the compiled CJS output couldn't be inspected; if dist/index.js contains a top‑level require('ethers') it will throw at runtime.
Options (choose one):
- Publish ESM‑only: remove the CJS "main"/"require" export so consumers import the ESM build (dist‑esm).
- Keep CJS but make the CJS entry a shim that does NOT top‑level require('ethers'') — lazy‑load via dynamic import(...) or proxy the API so CJS consumers don't require an ESM package synchronously.
- Provide a CJS‑friendly shim that avoids requiring the ESM‑only dependency.
8-8: Don't keep "sideEffects": false while any packaged module has import-time side-effects — remove or whitelist them.Scan found import-time side effects; action needed:
Paths with import-time side effects (representative):
- testing-functions/test.js — top-level require('dotenv').config().
- tests/** (e.g., tests/unitTests/.../validation.test.js, tests/unitTests/api_client/client.test.ts) — top-level dotenv imports and imports of ../dist/index.
- test-eth-app/src/App.tsx, test-eth-app/src/main.tsx — side-effect CSS imports (import './App.css', import './index.css') and browser-only window.ethereum usage.
Fix options (pick one):
- Ensure test/example files are excluded from the published package (npm "files" or .npmignore) and then keep "sideEffects": false.
- Otherwise remove/move import-time effects (move dotenv.config() to test setup scripts, avoid top-level CSS or global mutations in library entrypoints).
- Or explicitly list intentional side-effectful files in package.json "sideEffects".
resolves: https://github.com/trufnetwork/truf-network/issues/1205
Summary by CodeRabbit
New Features
Chores