Interim fix: remove Node crypto from level-private-state-provider browser path#789
Conversation
|
Hey guys, I saw that 2 reviewers added some commits to the PR, and several CI steps failed as they require passwords. Do I have to do anything else on my side? any comments on the submitted PR? |
|
Thanks for updating the branch. I see that My read is that the right conflict resolution is to prefer the current Before I update the branch: do you want me to resolve the conflicts in that direction, or is this PR now superseded by the changes already landed on For context, the original downstream validation was:
|
|
Thank you for your proposal @apestchanker . We have discussed this and implemented a more permanent solution. Removed node crypto, used web crypto and as a fallback: your proposal - noble. It's already on main and will be in next release. I hope this helps! |
Problem
@midnight-ntwrk/midnight-js-level-private-state-providercurrently imports Nodecryptodirectly in its runtime package code. In a browser app, that causes bundlers to fail or externalize the module, which prevents the provider from working in browser environments.In a downstream browser-targeted app built with Vite, this showed up as the browser build breaking on
cryptoresolution before the provider could even be exercised.This is not specific to Vite. Vite was simply the downstream browser build environment used for reproduction and validation.
The underlying issue is broader: the package currently imports Node-only runtime APIs in code that may be consumed in browser-targeted bundles. Any browser build pipeline can run into this class of incompatibility unless those imports are removed, conditionally exported, or otherwise replaced with browser-safe implementations.
Reproduction
Using a browser-targeted app that imports and instantiates:
and then uses:
the bundled app fails because the published runtime package imports Node
cryptodirectly.The concrete runtime/package issue is in:
packages/level-private-state-provider/src/storage-encryption.tspackages/level-private-state-provider/src/level-private-state-provider.tsBoth rely on Node
crypto, which is not safe to assume in a browser bundle.Summary
This is an interim browser-compatibility fix for
@midnight-ntwrk/midnight-js-level-private-state-provider.It replaces the runtime package's direct Node
cryptousage with noble-based browser-safe primitives so the package can be bundled in a browser app without immediately failing oncryptoresolution.Maintainer Note
This PR supersedes closed PR #787 and replaces PR #788.
I closed #787 after identifying a potential fresh-install compatibility issue for new consumers:
@noble/hashes@^2.xexports./sha2.js, not./sha256.js.I am also replacing #788 so the contribution history is clean for CLA/review automation and contains only commits under my GitHub identity.
This replacement PR keeps the same tested interim fix and only adds the minimum follow-up correction to those import paths so the noble dependency range and source imports stay aligned for new users.
What This Fixes
cryptofrompackages/level-private-state-providerStorageEncryptionWhy This Is Submitted As An Interim Fix
This patch addresses the first hard browser breakage: direct Node
cryptoimports in the published runtime package.In downstream validation, an additional app-side browser resolution step was still required for
eventsbecause thelevel/abstract-levelbrowser path is not fully packaged as a browser-native experience by the package alone.So this PR is intentionally scoped as a partial fix, not a claim of complete browser support.
Temporary Downstream Workaround
This PR does not fully solve browser packaging for the
level/abstract-levelstack by itself.In downstream validation using Vite, an additional browser resolution step was still required for
eventsbecauseabstract-levelextendsEventEmitter.That suggests the remaining issue is broader browser packaging for the
level/abstract-levelstack, not something unique to Vite itself.Example Vite workaround:
And install the browser-compatible
eventspackage:This workaround was enough in the downstream app to get past the
abstract-level/EventEmitterissue after removing the direct Nodecryptodependency.That is why this PR is submitted as an interim fix rather than a full browser-support solution.
Downstream Validation
Validated in a downstream browser-targeted app using this package for Midnight private state:
cryptoPermanent Follow-up Recommended
A more complete long-term solution likely needs a larger browser-specific implementation, for example:
level/abstract-levelstackTesting
What I was able to validate:
cryptoWhat I could not run in this environment:
If maintainers prefer, I can follow up with a larger browser-native / WebCrypto-oriented PR after alignment on the intended package support model.