Skip to content
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

chore(crypto-js): Add a release workflow for crypto-js #966

Merged
merged 3 commits into from
Aug 23, 2022

Conversation

Hywan
Copy link
Member

@Hywan Hywan commented Aug 22, 2022

This patch adds a new npm run publish script that:

  1. Run npm run prepublish (which runs the build and test scripts),
  2. Run wasm-pack publish.

Note 1: The prepublish script is using the $npm_execpath
environment variable instead of just “npm”, so that if someone is
using yarn or another JavaScript package manager, it should work
(not tested yet).

Note 2: wasm-pack publish is run without running wasm-pack login
before that. But we are updating the registry URL in the NPM
configuration file in the Github Action workflow, see below.

This patch then creates a new Github Action workflow that is triggered
when a new tag of the form matrix-sdk-crypto-js-v[0_9]+.* is
pushed. This workflow runs npm run publish basically, but before
that, it updates the NPM configuration file by setting a value for
//registry.npmjs.org/:_authToken. Thus, running wasm-pack login is
not necessary.

Supersedes and closes #900. Fixes #794.

Edit 🆕:

This patch updates the publish NPM script to explicitly run
wasm-pack pack to create the NPM package in the pkg/
directory. This patch also updates the Github Action workflow for the
matrix-sdk-crypto-js release, to create a new Github Release, with
the NPM package attached as an asset file.

This patch adds a new `npm run publish` script that:

1. Run `npm run prepublish` (which runs the `build` and `test` scripts),
2. Run `wasm-pack publish`.

Note 1: The `prepublish` script is using the `$npm_execpath`
environment variable instead of just “`npm`”, so that if someone is
using `yarn` or another JavaScript package manager, it _should_ work
(not tested yet).

Note 2: `wasm-pack publish` is run without running `wasm-pack login`
before that. _But_ we are updating the registry URL in the NPM
configuration file in the Github Action workflow, see below.

This patch then creates a new Github Action workflow that is triggered
when a new tag of the form `matrix-sdk-crypto-js-v[0_9]+.*` is
pushed. This workflow runs `npm run publish` basically, but before
that, it updates the NPM configuration file by setting a value for
`//registry.npmjs.org/:_authToken`. Thus, running `wasm-pack login` is
not necessary.
@Hywan Hywan added enhancement New feature or request bindings labels Aug 22, 2022
@Hywan Hywan requested a review from gnunicorn August 22, 2022 10:15
@Hywan
Copy link
Member Author

Hywan commented Aug 22, 2022

The differences with #900 are:

  • wasm-pack is run from an NPM script, so the wasm-pack's version installed by NPM, i.e. that lands in bindings/matrix-sdk-crypto-js/node_modules/, along with wasm-opt etc. Everything is embedded.

  • wasm-pack publish is used to publish the package on NPM, which runs npm publish behind the scene. We don't use any other Github Actions, like JS-DevTools/npm_publish which was used in Crypto-JS release infrastructure #900. Problem is: How to pass an auth token, because we don't want to run wasm-pack login, which runs npm login, which is an interactive command that reads the login and the password? We have a token! Well, my trick is to update the NPM configuration key //registry.npmjs.org/:_authToken to set our token as the key value here. That's what JS-DevTools/npm-publish is doing. Less code, less dependencies, less friction. I just hope it will work!

Copy link
Contributor

@gnunicorn gnunicorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but I have some questions...

@codecov
Copy link

codecov bot commented Aug 22, 2022

Codecov Report

Merging #966 (331f8b3) into main (68107e6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #966   +/-   ##
=======================================
  Coverage   80.35%   80.35%           
=======================================
  Files         110      110           
  Lines       15809    15809           
=======================================
  Hits        12704    12704           
  Misses       3105     3105           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Hywan added 2 commits August 22, 2022 14:37
…ched.

This patch updates the `publish` NPM script to explicitly run
`wasm-pack pack` to create the NPM package in the `pkg/`
directory. This patch also updates the Github Action workflow for the
`matrix-sdk-crypto-js` release, to create a new Github Release, with
the NPM package attached as an asset file.
@Hywan Hywan merged commit d508237 into matrix-org:main Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bindings enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(bindings/crypto-js): Ship WASM artifact to npm
2 participants