fix: remove implicit bitcoinjs require from DescriptorsFactory#67
Closed
Kukks wants to merge 1 commit intobitcoinerlab:mainfrom
Closed
fix: remove implicit bitcoinjs require from DescriptorsFactory#67Kukks wants to merge 1 commit intobitcoinerlab:mainfrom
Kukks wants to merge 1 commit intobitcoinerlab:mainfrom
Conversation
Remove the lazy `require('./adapters/bitcoinjs')` fallback that fired
when DescriptorsFactory received a raw TinySecp256k1Interface instead
of a pre-built BitcoinLib adapter.
The require chain (descriptors.ts → adapters/bitcoinjs.ts → bitcoinjs-lib)
caused bundlers like esbuild (Cloudflare Wrangler), Webpack, and Metro to
statically resolve bitcoinjs-lib even when consumers only use the scure
backend. This broke builds where bitcoinjs-lib is not installed.
DescriptorsFactory now requires an explicit backend adapter:
- DescriptorsFactory(createBitcoinjsLib(ecc))
- DescriptorsFactory(createScureLib())
The convenience packages (@bitcoinerlab/descriptors, descriptors-scure)
already pass pre-built adapters, so they are unaffected.
Ref: bitcoinerlab#66
Member
|
Thanks for the PR. I have an alternative non-breaking fix that will be merged today, @Kukks. I'd like to verify it doesn't affect downstream dependents before proceeding further. |
Member
|
Thanks again! Closed in favour of #68 . Published as |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
require('./adapters/bitcoinjs')fallback fromDescriptorsFactorythat caused bundler failures whenbitcoinjs-libis not installedDescriptorsFactorynow requires an explicitBitcoinLibbackend adapter (createBitcoinjsLib(ecc)orcreateScureLib()) instead of accepting a rawTinySecp256k1Interface@bitcoinerlab/descriptors,@bitcoinerlab/descriptors-scure) already use explicit adapters and are unaffectedProblem
The
require('./adapters/bitcoinjs')inDescriptorsFactorycreates a static dependency chain:Bundlers like esbuild (Cloudflare Wrangler), Webpack, and Metro resolve this chain statically — even inside
try/catchblocks — causing build failures whenbitcoinjs-libis not installed. This affects consumers who only use the scure backend.Converting
require()toimport()does not help because with"module": "commonjs"in tsconfig, TypeScript compilesimport()toPromise.resolve().then(() => require(...)), which bundlers still follow (as documented inledger.tslines 70-81).Solution
Remove the implicit fallback entirely. Consumers must now explicitly choose their backend:
This is a minor breaking change for consumers who used the implicit
DescriptorsFactory(ecc)shorthand directly ondescriptors-core. The convenience wrapper packages are unaffected since they already pass pre-built adapters.Note on remaining
require()callsThe other
require()calls in the codebase (psbt.ts,keyInterfaces.ts,signers.ts,ledger.ts) resolve to internal adapter files that are always present in the package, so they don't cause bundler failures. Theledger.tsrequire('@ledgerhq/ledger-bitcoin')is only reached when consumers explicitly use Ledger functionality.Ref: #66