fix: add subpath exports, deprecate namespace re-exports for turbopack compat#987
fix: add subpath exports, deprecate namespace re-exports for turbopack compat#987
Conversation
📝 WalkthroughWalkthroughPackage export surface expanded with new subpath exports and CTS/MTS type entry points. Namespace re-exports replaced by deprecation proxies in the index. RelayFailedError/RelayExplorer type declarations moved and re-exported; a few imports and examples updated to use the new submodule packages. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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.
🧹 Nitpick comments (1)
connect/src/protocols/tokenBridge/tokenTransfer.ts (1)
61-75: Merge split imports from the same module.
RelayFailedErroron Line 75 is imported from../../types.js— the exact same specifier already used for the value import block on Lines 61–69. Consolidate them to avoid the redundant module reference.♻️ Proposed consolidation
import { TransferState, isAttested, isInReview, isRedeemed, isRelayFailed, isSourceFinalized, isSourceInitiated, + RelayFailedError, } from "../../types.js"; import { getGovernedTokens, getGovernorLimits } from "../../whscan-api.js"; import { Wormhole } from "../../wormhole.js"; import type { WormholeTransfer } from "../wormholeTransfer.js"; import type { QuoteWarning } from "../../warnings.js"; import { RelayStatus } from "@wormhole-foundation/sdk-definitions"; -import { RelayFailedError } from "../../types.js";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connect/src/protocols/tokenBridge/tokenTransfer.ts` around lines 61 - 75, The file imports several symbols from ../../types.js in one block (TransferState, isAttested, isInReview, isRedeemed, isRelayFailed, isSourceFinalized, isSourceInitiated) and then re-imports RelayFailedError from the same module; consolidate these into a single import from ../../types.js that includes RelayFailedError along with TransferState and the is* helpers to remove the redundant module reference and keep imports grouped (update the import that currently lists TransferState... to also include RelayFailedError and remove the separate RelayFailedError import).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@connect/src/protocols/tokenBridge/tokenTransfer.ts`:
- Around line 61-75: The file imports several symbols from ../../types.js in one
block (TransferState, isAttested, isInReview, isRedeemed, isRelayFailed,
isSourceFinalized, isSourceInitiated) and then re-imports RelayFailedError from
the same module; consolidate these into a single import from ../../types.js that
includes RelayFailedError along with TransferState and the is* helpers to remove
the redundant module reference and keep imports grouped (update the import that
currently lists TransferState... to also include RelayFailedError and remove the
separate RelayFailedError import).
75d9822 to
a5634ee
Compare
…k compat Turbopack's scope hoisting generates broken module wrappers for namespace re-exports, causing "module factory is not available" hydration crashes. - Add subpath exports (./routes, ./tasks, ./circle-api, ./whscan-api) - Wrap existing namespace re-exports in deprecation Proxies that emit a one-time console.warn directing consumers to subpath imports - Move RelayFailedError from routes/types.ts to types.ts to break circular dependency; re-export from routes/types.ts for compat - Fix self-referential import in tokenTransfer.ts - Update examples to use new subpath imports
a5634ee to
7125459
Compare
kev1n-peters
left a comment
There was a problem hiding this comment.
Should we publish a beta version with this change and verify it works in Connect?
The deprecation Proxy change (7125459) broke TypeScript namespace access: tsc emits `export declare const routes: typeof _routes` which consumers can't use as a namespace (TS2503). Add hand-written index.d.cts/index.d.mts at the package root with proper `export * as routes` namespace re-exports. The `exports.types` condition redirects consumers to these files while the runtime JS keeps the Proxy. Paths reference dist/ so resolution works in both the source repo and published package.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
connect/src/types.ts (1)
16-22: Nit:this.nameis not set in the custom Error subclass.
RelayFailedErrorinheritsname = "Error"fromError.prototype, so stack traces / serialization won't show the subclass name. The same pattern exists inMinAmountErrorandUnavailableErrorinroutes/types.ts, so this is consistent — but worth noting as a potential future improvement across all custom error classes.Optional improvement
export class RelayFailedError extends Error { readonly relayExplorer?: RelayExplorer; constructor(message: string, relayExplorer?: RelayExplorer) { super(message); + this.name = "RelayFailedError"; this.relayExplorer = relayExplorer; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connect/src/types.ts` around lines 16 - 22, Custom error subclasses (RelayFailedError, and similarly MinAmountError and UnavailableError) do not set this.name, so stack traces and serialization show "Error" instead of the subclass; update each constructor (e.g., RelayFailedError.constructor) to assign this.name = "RelayFailedError" (and correspondingly "MinAmountError"/"UnavailableError") immediately after calling super(message) so the error instances carry the correct subclass name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@connect/src/types.ts`:
- Around line 16-22: Custom error subclasses (RelayFailedError, and similarly
MinAmountError and UnavailableError) do not set this.name, so stack traces and
serialization show "Error" instead of the subclass; update each constructor
(e.g., RelayFailedError.constructor) to assign this.name = "RelayFailedError"
(and correspondingly "MinAmountError"/"UnavailableError") immediately after
calling super(message) so the error instances carry the correct subclass name.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
connect/index.d.ctsconnect/index.d.mtsconnect/package.jsonconnect/src/index.tsconnect/src/protocols/tokenBridge/tokenTransfer.tsconnect/src/routes/types.tsconnect/src/types.tsexamples/src/quorum.tsexamples/src/router.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/src/router.ts
- connect/src/protocols/tokenBridge/tokenTransfer.ts
- examples/src/quorum.ts
Summary
./routes,./tasks,./circle-api,./whscan-api) toconnect/package.jsonroutes,tasks,circleApi,api) in deprecation Proxies that emit a one-timeconsole.warndirecting consumers to use subpath imports@deprecatedJSDoc tags for IDE strikethrough hintsRelayFailedErrorfromroutes/types.tstotypes.tsto break circular dependency; re-export fromroutes/types.tsfor backward compatimport { routes } from "../../index.js"intokenTransfer.tsNon-Breaking
Existing imports continue to work — consumers will see a deprecation warning:
Migration
Same pattern for
tasks→./tasks,circleApi→./circle-api,api→./whscan-api.Context
Turbopack (default bundler in Next.js 15+) has a scope hoisting bug where namespace re-exports generate a wrapper module whose factory is never emitted into client chunks. SSR works fine but client hydration crashes with "module factory is not available". This affects all syntax variants.
Consumers using turbopack must use the subpath imports to avoid the crash. The deprecation warnings guide all consumers toward the new pattern ahead of a future removal.
Tracked in Next.js: vercel/next.js#82827
Test plan
cd connect && npm run buildpasses (pre-existing type errors on main unrelated to this PR)cd connect && npm run testpassesimport { routes } from "@wormhole-foundation/sdk-connect"still works (logs deprecation warning)import * as routes from "@wormhole-foundation/sdk-connect/routes"resolves correctlyRelayFailedErroraccessible from bothtypesandroutes/typespathsSummary by CodeRabbit