Preserve ESM for async imports to work correctly

[ms-dosx86]

trafficstars

Checklist

• [ ] Tests written for new code (and old code if feasible).
• [ ] New or updated public/exported symbols have accurate TSDoc documentation.
• [ ] Linter and other CI checks pass.
• [ ] Sign-off given on the changes (see CONTRIBUTING.md).

Fixes https://github.com/matrix-org/matrix-js-sdk/issues/4154

There were a couple of things I encountered during this PR:

1. I had to add import type in src/models/MSC3089TreeSpace.ts because @babel/preset-typescript refused to remove that unused import. It contains two interfaces that should be removed after yarn build. But I guess this https://github.com/matrix-org/matrix-js-sdk/blob/develop/src/models/MSC3089TreeSpace.ts#L83

declare module "../@types/media" {
    interface FileContent {
        [UNSTABLE_MSC3089_LEAF.name]?: {};
    }
}

prevents it from being removed. import type fixes this with minimum amount of effort.

2. jest refused to run spec files with ESM enabled with babel-jest. It kept throwing this error

SyntaxError: Cannot use import statement outside a module

even tho I have babel and babel-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 of babel-jest. It worked but 7 of 137 specs failed. This is one of them

at Object.<anonymous> (src/client.ts:235:57)
at Object.<anonymous> (src/models/thread.ts:19:1)
at Object.<anonymous> (src/models/event.ts:39:1)
at Object.<anonymous> (src/crypto/EncryptionSetup.ts:18:1)
at Object.<anonymous> (src/crypto/index.ts:36:1)
at Object.<anonymous> (spec/unit/crypto.spec.ts:7:1)

According to this https://github.com/kulshekhar/ts-jest/issues/1873 it might be related to circular dependencies problem since src/crypto/index.ts ends up importing src/client.ts which tries to import ./crypto so the test fails. It works fine with yarn build tho. I'm not familiar with jest so any help here will be appreciated.

Anyway, I replaced .babelrc with babel.config.js to run tests with modules: "commonjs" to make jest 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 example

npx esbuild examples/lazy-crypto-init/index.js --bundle --splitting --outdir=examples/lazy-crypto-init --format=esm --servedir=examples/lazy-crypto-init --watch --define:global=globalThis

[richvdh]

2. jest refused to run spec files with ESM enabled with babel-jest. It kept throwing this error

SyntaxError: Cannot use import statement outside a module

Yup. It looks like you're hitting https://github.com/jestjs/jest/issues/9860. As that issue says (eventually), you can solve the problem by adding extensionsToTreatAsEsm: ['.ts'] to the jest config.

However then you discover the next problem, which is that our tests rely on jest.mock to mock out entire modules, which doesn't seem to work very well either. (https://github.com/jestjs/jest/issues/10025 seems related, though it doesn't entirely match the behaviour I observe).

Anyway, I think your proposed approach of sticking to CJS for Jest is the right one, for now at least.

[ms-dosx86]

Hi @richvdh !

• [x] I agree that the example is not necessary and removed it.
• [x] Also added a comment as you asked. Hope you don't mind mentioning your comment there since you explained the situation very clearly.
• [x] And added a sign off in the last commit. Sorry, that was my bad. Didn't see it in the description checklist.

[richvdh]

And added a sign off in the last commit. Sorry, that was my bad. Didn't see it in the description checklist.

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 line there?

[ms-dosx86]

And added a sign off in the last commit. Sorry, that was my bad. Didn't see it in the description checklist.

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 line there?

Oh.. I see. I updated the description. Hope that it's correct this time.