Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR centralizes package metadata by adding exports Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bin/lib/package.json`:
- Around line 48-52: The package is unconditionally exporting `@typeberry/native`
to browsers via the `@typeberry/lib` "./crypto" subpath which re-exports symbols
(bandersnatch, initAll) from `@typeberry/native`; update the package exports so
native bindings are not exposed to browser consumers by either (A) changing the
./crypto export to a conditional subpath (e.g., provide a node-specific export
like "./crypto": {"node": "./crypto.node.js", "default": "./crypto.browser.js"})
or (B) remove `@typeberry/native` from the public exports and import it only
internally in `@typeberry/crypto`, exposing a pure-JS/browser-safe facade from
`@typeberry/lib/crypto`; ensure references to bandersnatch and initAll are only
resolved by the node-specific module or internal import.
🧹 Nitpick comments (1)
bin/jam/args.ts (1)
6-6: Consider using@typeberry/utilsfor consistency with other CLI entry points.While importing the top-level package.json is allowed per coding guidelines, other CLI files in this PR (e.g.,
bin/rpc/main.ts,bin/tci/args.ts,bin/convert/args.ts) now importversionfrom@typeberry/utils. Consider aligning this file with the same pattern for consistency across the codebase.♻️ Optional refactor for consistency
-import packageJson from "../../package.json" with { type: "json" }; +import { version } from "@typeberry/utils";And update line 18:
-@typeberry/jam ${packageJson.version} by Fluffy Labs. +@typeberry/jam ${version} by Fluffy Labs.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.coderabbit.yaml:
- Around line 74-77: Update the rule in the .coderabbit.yaml block that
currently replaces package.json imports with imports from `@typeberry/utils` to
either (A) add a documented exception for browser-only packages or (B) clarify
that the rule targets server/universal packages and relies on tree-shaking;
mention the browser-package exception by name and explain when to keep direct
package.json imports, and reference the Node-specific symbols in
`@typeberry/utils` (debug.ts: process.hrtime, env.ts: process.env, test.ts:
node:assert) so maintainers know why the exception exists and when to apply it.
View all
Benchmarks summary: 83/83 OK ✅ |
Importing
package.jsonfiles causes them to be included in the built library, which in turn breaks theimportsoverrides defined in the top-level package.json only (i.e. nested package.json creates a local scope which does not haveimports, hence it breaks).The issues is not present when sub-exports are used and the affected packages are not imported, but it always appears when a default, top-level import is used.
Relying on a single
package.jsonimport in utils should resolve that issue, since none of the nestedpackage.jsons will be included.