-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Publish WASM package to enable argon2 support on CLI #691
Conversation
@@ -15,6 +15,7 @@ | |||
"main": "node/bitwarden_wasm.js", | |||
"module": "index.js", | |||
"types": "bitwarden_wasm.d.ts", | |||
"scripts": {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't technically required, but npm would output a confusing warning if it wasn't there, so I decided to add it.
New Issues
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #691 +/- ##
==========================================
- Coverage 60.83% 60.71% -0.13%
==========================================
Files 173 173
Lines 10598 10619 +21
==========================================
Hits 6447 6447
- Misses 4151 4172 +21 ☔ View full report in Codecov by Sentry. |
# Conflicts: # crates/bitwarden-wasm/Cargo.toml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can get rid of the SETUP NPM by using the actions/setup-node
action with registry-url
per https://docs.github.com/en/actions/publishing-packages/publishing-nodejs-packages#publishing-packages-to-the-npm-registry. You can also omit writing the token to disk and just pass it in as an env during the publish command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm hesitant to change this here as all our other NPM publishing workflows in sdk and clients are doing it this way, so I wouldn't want a single workflow being different than the rest.
I definitely think this is a good improvement over manually creating the file, and we definitely want to switch to it, but I think it would be better for a separate PR to update all workflows at once.
Type of change
Objective
Continuation of #680
I've created workflows to publish the WASM builds to NPM, so that it can be used from the CLI client.
The publishing step is based on the NAPI builds, and I've tested it against the Github NPM registry. We'll need to get a build going and published on the NPM registry to be able to use it on the CLI.