feat: add explicit register() exports for tree-shaking compatibility#825
feat: add explicit register() exports for tree-shaking compatibility#825
Conversation
Add idempotent register() function exports to definitions, evm, solana, and sui packages so consumers can use named imports instead of bare side-effect imports. This gives bundlers (webpack/turbopack) a bound import to preserve when sideEffects optimization is enabled. Module-scope auto-registration is preserved for backward compatibility.
📝 WalkthroughWalkthroughReplaced top-level side-effect registrations with explicit, idempotent exported Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
evm/ts/src/index.ts (1)
1-34:⚠️ Potential issue | 🟡 MinorFix Prettier formatting issue flagged by CI.
The pipeline reports a code style violation in this file. Run
prettier --write evm/ts/src/index.tsto resolve.
🧹 Nitpick comments (1)
solana/ts/sdk/index.ts (1)
5-21: The bare side-effect import on Line 6 is a valid concern — verify if it should be incorporated intoregister().The
import "./side-effects"on Line 6 contains critical runtime patches that suppress console warnings from native libraries (secp256k1andbigint). This bare side-effect import could be tree-shaken away by bundlers configured withsideEffects: false, causing those warnings to unexpectedly appear in consumers' output.Since this PR aims to move away from side-effect imports, either incorporate the side-effects module into the
register()function to ensure it always runs when the SDK is initialized, or ensure the file is marked as non-tree-shakeable in the package configuration.
Check the actual SDK registry before attempting registration rather than relying on a module-level _registered flag. This verifies the protocols are actually registered, not just that the function ran.
Module-scope register() calls now emit console.warn when triggered
as a side effect of importing the package, nudging consumers to
import { register } and call it explicitly.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@evm/ts/src/index.ts`:
- Around line 38-40: The EVM package is auto-calling register(true) which causes
duplicate deprecation warnings when the definitions package (via
registerDefinitions()) has already performed its own register(true); update the
EVM top-level import logic to avoid emitting a second warning by replacing the
auto-call register(true) with a non-warning call or no-op: either remove the
module-scope register(true) call entirely or change it to register(false)/a
suppressed form so registerDefinitions() still runs but no duplicate deprecation
warning is logged; reference the symbols register(true) in this file and
registerDefinitions() to locate and modify the behavior.
In `@sdk/definitions/src/index.ts`:
- Around line 12-32: The unconditional auto-registration call register(true)
triggers the deprecation warning for every importer; change the auto-register
invocation to suppress the warning by calling register() or register(false)
instead, or alternatively wrap the register(true) call in a NODE_ENV check (e.g.
only call it when process.env.NODE_ENV !== 'production') so production consumers
aren't noisy; apply the same change to the equivalent auto-registration calls
that use register(true) across the other SDK entry points (the EVM, Solana and
Sui module entry files) to keep behavior consistent.
Summary
register()function exports to definitions, evm, solana, and sui NTT SDK packagesimport { register } from '...') instead of bare side-effect imports (import '...')sideEffectsoptimization is enabledWhy
wormhole-connect's
sideEffectsfield optimization causes webpack/turbopack to drop bare side-effect imports (import '@wormhole-foundation/sdk-evm-ntt'). With a namedregister()export, consumers create a bound import that bundlers preserve regardless ofsideEffectssettings.Changes
sdk/definitions/src/index.tsregister()wrappingregisterPayloadTypes()callsevm/ts/src/index.tsregister()wrappingregisterProtocol()calls + definitions registrationsolana/ts/sdk/index.tsregister()wrappingregisterProtocol()calls + definitions registrationsui/ts/src/index.tsregister()wrappingregisterProtocol()calls + definitions registrationBackward Compatibility
import "@wormhole-foundation/sdk-evm-ntt"(bare)register()at module scope → same resultimport { EvmNtt } from "@wormhole-foundation/sdk-evm-ntt"register()at module scope → same resultimport { register } from "..."; register()register()is idempotent no-opTest plan
bun run build— definitions, solana, sui compile. EVM has pre-existingZeroGravitytype error (unrelated)registerexport confirmed indist/esm/index.jsfor definitions, solana, suibun run test)Summary by CodeRabbit
New Features
Bug Fixes