paulmillr / scure-btc-signer

Audited & minimal library for creating, signing & decoding Bitcoin transactions.
https://paulmillr.com/noble/#scure
MIT License
155 stars 40 forks source link

Future refactor: Remove circular imports #110

Open cmdruid opened 2 months ago

cmdruid commented 2 months ago

Hello. I'm running into an issue where there's a circular import in this package that leads to rollup.js breaking when trying to bundle for the browser:

[!] Error: Circular dependency: node_modules/@scure/btc-signer/lib/esm/payment.js -> node_modules/@scure/btc-signer/lib/esm/psbt.js -> node_modules/@scure/btc-signer/lib/esm/transaction.js -> node_modules/@scure/btc-signer/lib/esm/payment.js
    at onwarn (file:///home/cscott/Projects/ducat/client-sdk/rollup.config-1724172435327.mjs:20:9)
    at /home/cscott/Projects/ducat/client-sdk/node_modules/rollup/dist/shared/rollup.js:655:13
    at /home/cscott/Projects/ducat/client-sdk/node_modules/rollup/dist/shared/rollup.js:641:32
    at Object.logger [as onLog] (/home/cscott/Projects/ducat/client-sdk/node_modules/rollup/dist/shared/rollup.js:1120:9)
    at Graph.sortModules (/home/cscott/Projects/ducat/client-sdk/node_modules/rollup/dist/shared/rollup.js:20637:26)
    at Graph.build (/home/cscott/Projects/ducat/client-sdk/node_modules/rollup/dist/shared/rollup.js:20545:14)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at /home/cscott/Projects/ducat/client-sdk/node_modules/rollup/dist/shared/rollup.js:21146:13
    at catchUnfinishedHookActions (/home/cscott/Projects/ducat/client-sdk/node_modules/rollup/dist/shared/rollup.js:20698:16)
    at rollupInternal (/home/cscott/Projects/ducat/client-sdk/node_modules/rollup/dist/shared/rollup.js:21141:5)

I see that you have this commented in the code as a circular reference, do you have any plans to fix it in the future?

paulmillr commented 2 months ago

In a big future refactoring that would be nice to fix.

That seems like something rollup should handle. Perhaps your configuration is wrong. I've briefly searched and they seem to support circular imports.

cmdruid commented 2 months ago

Rollup isn't able to fix the circular references, but I looked into it more and the warning can be suppressed. I went ahead with that route, and hopefully the bundling works without issue. Thanks for the response and clarity.