-
-
Notifications
You must be signed in to change notification settings - Fork 609
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
Preserve ESM for async imports to work correctly #4187
Preserve ESM for async imports to work correctly #4187
Conversation
Yup. It looks like you're hitting jestjs/jest#9860. As that issue says (eventually), you can solve the problem by adding However then you discover the next problem, which is that our tests rely on Anyway, I think your proposed approach of sticking to CJS for Jest is the right one, for now at least. |
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.
Please could you also add a sign-off to your PR, as requested in https://github.com/element-hq/element-web/blob/develop/CONTRIBUTING.md#sign-off
Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com>
Hi @richvdh !
|
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.
minor wording suggestions
Thanks. Unfortunately that doesn't help with the earlier commits. Could you edit the description of the pull request to add a |
Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Oh.. I see. I updated the description. Hope that it's correct this time. |
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.
LGTM! Thanks!
This example seems to have been broken by the switch to Typescript. We can't just symlink in `../..` because that gives us the typescript version of the source, which, obviously, doesn't work in node. Instead, make sure we use a prebuilt version of the js-sdk. It's actually even more broken as of js-sdk 33.0.0, thanks to the switch to ES modules (#4187), but we'll get to that later.
This example seems to have been broken by the switch to Typescript. We can't just symlink in `../..` because that gives us the typescript version of the source, which, obviously, doesn't work in node. Instead, make sure we use a prebuilt version of the js-sdk. It's actually even more broken as of js-sdk 33.0.0, thanks to the switch to ES modules (#4187), but we'll get to that later.
* Preserve ESM for async imports to work correctly (matrix-org#4187) * fix: fix lazy rust crypto import * test: use "commonjs" for tests because of circular deps * chore: revert commonjs for "module" * refactor: remove unnecessary example * refactor: add comments Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> * refactor: improve comment Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update babel.config.js --------- Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update organization (matrix-org#4212) * v32.4.0 * Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0 (matrix-org#4214) * Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0 * Disable affected test Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> * `initRustCrypto`: allow app to pass in the store key directly (matrix-org#4210) * `initRustCrypto`: allow app to pass in the store key directly ... instead of using the pickleKey. This allows us to avoid a slow PBKDF operation. * Fix link in doc-comment * Bump matrix-sdk-crypto-wasm to 5.0.0 (matrix-org#4216) Slightly more involved than normal because it requires us to pass a backup version into OlmMachine.importBackedUpRoomKeys. On the other hand we can now re-enable the test that was disabled in matrix-org#4214 due to matrix-org/matrix-rust-sdk#3447 Fixes: element-hq/element-web#27165 * Remove more deprecated methods, fields, and exports (matrix-org#4217) * Don't run migration for Rust crypto if the legacy store is empty (matrix-org#4218) * Don't run migration for Rust crypto if the legacy store is empty Fixes element-hq/element-web#27447 * Add copyright for the TypeScript files in legacy DB dumps * Provide a type for the accountPickle we check for before migration * Remove redundant backup response This is unused * Simplify keys response * Downgrade log message. --------- Co-authored-by: Richard van der Hoff <richard@matrix.org> * v33.0.0-rc.0 * v33.0.0 --------- Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: R Midhun Suresh <hi@midhun.dev> Co-authored-by: RiotRobot <releases@riot.im> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Andy Balaam <andy.balaam@matrix.org> Co-authored-by: Richard van der Hoff <richard@matrix.org>
* Preserve ESM for async imports to work correctly (matrix-org#4187) * fix: fix lazy rust crypto import * test: use "commonjs" for tests because of circular deps * chore: revert commonjs for "module" * refactor: remove unnecessary example * refactor: add comments Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> * refactor: improve comment Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update babel.config.js --------- Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update organization (matrix-org#4212) * v32.4.0 * Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0 (matrix-org#4214) * Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0 * Disable affected test Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> * `initRustCrypto`: allow app to pass in the store key directly (matrix-org#4210) * `initRustCrypto`: allow app to pass in the store key directly ... instead of using the pickleKey. This allows us to avoid a slow PBKDF operation. * Fix link in doc-comment * Bump matrix-sdk-crypto-wasm to 5.0.0 (matrix-org#4216) Slightly more involved than normal because it requires us to pass a backup version into OlmMachine.importBackedUpRoomKeys. On the other hand we can now re-enable the test that was disabled in matrix-org#4214 due to matrix-org/matrix-rust-sdk#3447 Fixes: element-hq/element-web#27165 * Remove more deprecated methods, fields, and exports (matrix-org#4217) * Don't run migration for Rust crypto if the legacy store is empty (matrix-org#4218) * Don't run migration for Rust crypto if the legacy store is empty Fixes element-hq/element-web#27447 * Add copyright for the TypeScript files in legacy DB dumps * Provide a type for the accountPickle we check for before migration * Remove redundant backup response This is unused * Simplify keys response * Downgrade log message. --------- Co-authored-by: Richard van der Hoff <richard@matrix.org> * v33.0.0-rc.0 * v33.0.0 --------- Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: R Midhun Suresh <hi@midhun.dev> Co-authored-by: RiotRobot <releases@riot.im> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Andy Balaam <andy.balaam@matrix.org> Co-authored-by: Richard van der Hoff <richard@matrix.org>
* Bringing develop to v 33.0.0 (#5) * Preserve ESM for async imports to work correctly (matrix-org#4187) * fix: fix lazy rust crypto import * test: use "commonjs" for tests because of circular deps * chore: revert commonjs for "module" * refactor: remove unnecessary example * refactor: add comments Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> * refactor: improve comment Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update babel.config.js --------- Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update organization (matrix-org#4212) * v32.4.0 * Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0 (matrix-org#4214) * Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0 * Disable affected test Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> * `initRustCrypto`: allow app to pass in the store key directly (matrix-org#4210) * `initRustCrypto`: allow app to pass in the store key directly ... instead of using the pickleKey. This allows us to avoid a slow PBKDF operation. * Fix link in doc-comment * Bump matrix-sdk-crypto-wasm to 5.0.0 (matrix-org#4216) Slightly more involved than normal because it requires us to pass a backup version into OlmMachine.importBackedUpRoomKeys. On the other hand we can now re-enable the test that was disabled in matrix-org#4214 due to matrix-org/matrix-rust-sdk#3447 Fixes: element-hq/element-web#27165 * Remove more deprecated methods, fields, and exports (matrix-org#4217) * Don't run migration for Rust crypto if the legacy store is empty (matrix-org#4218) * Don't run migration for Rust crypto if the legacy store is empty Fixes element-hq/element-web#27447 * Add copyright for the TypeScript files in legacy DB dumps * Provide a type for the accountPickle we check for before migration * Remove redundant backup response This is unused * Simplify keys response * Downgrade log message. --------- Co-authored-by: Richard van der Hoff <richard@matrix.org> * v33.0.0-rc.0 * v33.0.0 --------- Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: R Midhun Suresh <hi@midhun.dev> Co-authored-by: RiotRobot <releases@riot.im> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Andy Balaam <andy.balaam@matrix.org> Co-authored-by: Richard van der Hoff <richard@matrix.org> * Bring develop to v33.0.0 (#6) * Preserve ESM for async imports to work correctly (matrix-org#4187) * fix: fix lazy rust crypto import * test: use "commonjs" for tests because of circular deps * chore: revert commonjs for "module" * refactor: remove unnecessary example * refactor: add comments Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> * refactor: improve comment Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update babel.config.js --------- Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update organization (matrix-org#4212) * v32.4.0 * Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0 (matrix-org#4214) * Update dependency @matrix-org/matrix-sdk-crypto-wasm to v4.10.0 * Disable affected test Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> * `initRustCrypto`: allow app to pass in the store key directly (matrix-org#4210) * `initRustCrypto`: allow app to pass in the store key directly ... instead of using the pickleKey. This allows us to avoid a slow PBKDF operation. * Fix link in doc-comment * Bump matrix-sdk-crypto-wasm to 5.0.0 (matrix-org#4216) Slightly more involved than normal because it requires us to pass a backup version into OlmMachine.importBackedUpRoomKeys. On the other hand we can now re-enable the test that was disabled in matrix-org#4214 due to matrix-org/matrix-rust-sdk#3447 Fixes: element-hq/element-web#27165 * Remove more deprecated methods, fields, and exports (matrix-org#4217) * Don't run migration for Rust crypto if the legacy store is empty (matrix-org#4218) * Don't run migration for Rust crypto if the legacy store is empty Fixes element-hq/element-web#27447 * Add copyright for the TypeScript files in legacy DB dumps * Provide a type for the accountPickle we check for before migration * Remove redundant backup response This is unused * Simplify keys response * Downgrade log message. --------- Co-authored-by: Richard van der Hoff <richard@matrix.org> * v33.0.0-rc.0 * v33.0.0 --------- Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: R Midhun Suresh <hi@midhun.dev> Co-authored-by: RiotRobot <releases@riot.im> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Andy Balaam <andy.balaam@matrix.org> Co-authored-by: Richard van der Hoff <richard@matrix.org> --------- Signed-off-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Bayyr Oorjak <the.bayyr.oorjak@gmail.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: R Midhun Suresh <hi@midhun.dev> Co-authored-by: RiotRobot <releases@riot.im> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> Co-authored-by: Andy Balaam <andy.balaam@matrix.org> Co-authored-by: Richard van der Hoff <richard@matrix.org>
Checklist
public
/exported
symbols have accurate TSDoc documentation.Fixes #4154
There were a couple of things I encountered during this PR:
import type
insrc/models/MSC3089TreeSpace.ts
because@babel/preset-typescript
refused to remove that unused import. It contains two interfaces that should be removed afteryarn build
. But I guess this https://github.com/matrix-org/matrix-js-sdk/blob/develop/src/models/MSC3089TreeSpace.ts#L83prevents it from being removed.
import type
fixes this with minimum amount of effort.jest
refused to run spec files with ESM enabled withbabel-jest
. It kept throwing this erroreven tho I have
babel
andbabel-typescript
configured properly. I tried as described in the docs https://jestjs.io/docs/ecmascript-modules , but no success.Then I tried
ts-jest
instead ofbabel-jest
. It worked but 7 of 137 specs failed. This is one of themAccording to this kulshekhar/ts-jest#1873 it might be related to circular dependencies problem since
src/crypto/index.ts
ends up importingsrc/client.ts
which tries to import./crypto
so the test fails. It works fine withyarn build
tho. I'm not familiar withjest
so any help here will be appreciated.Anyway, I replaced
.babelrc
withbabel.config.js
to run tests withmodules: "commonjs"
to makejest
work.And finally I added an example to test async imports (
rust-crypto
import) and see that they were splitted from the main chunk. A command to build and serve the exampleSigned-off-by: Bayyr Oorjak the.bayyr.oorjak@gmail.com