clients icon indicating copy to clipboard operation
clients copied to clipboard

[EC-648] Fix esm features in node testing environment

Open eliykat opened this issue 2 years ago • 0 comments

Type of change

- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

#3532 uses the import.meta syntax, which is an ES2020 feature. This is fine with browsers, but causes issues in our node testing environment - ES2020 support (and import.meta support specifically) is experimental (and unsupported) for node and Jest, not enabled by default.

I avoided this at the time by playing "keep away" with the import.meta statement, tweaking imports so that it would never be touched by test code. However, this is only a hack, and I want to solve it properly.

There are a few options here:

  1. Enable ES2020 support in node and Jest. See Jest docs for more info. Experimental, unsupported, requires various changes to our tests to get them working with these options. I tried it anyway but didn't have much luck.
  2. Some kind of transformation with babel? Unfortunately it's non-trivial to pipe output through both ts-jest and babel - ts-jest docs say babel is supported, but (as far as I can tell) the use case here is to use ts-jest for *.ts and babel for *.js, it's not really anticipated that you use them both sequentially. Plus this would be a lot of double handling, it's a pretty heavyweight solution. (I also tried dropping ts-jest and compiling everything with babel, which would be cool, but seems extremely unsupported!)
  3. Write a custom AST transformer to remove the offending syntax - this is supported by ts-jest and didn't actually turn out to be that difficult. That's what I went with in the end.

Code changes

  • libs/shared/es2020-transformer.ts - the transformer, based on the resources I've linked to in the code comment. (please refer to those for more info.) It traverses each node of the AST (Abstract Syntax Tree, a representation of the source code), and if it finds an import.meta statement, it'll remove it. This happens before ts-jest does its thing.
  • jest.config.js - update each Angular project to use this transformer. I did this in each individual jest config so that I could specify the correct relative path to the transformer. There is no variable to refer to the root of the monorepo (or of jest.config.base.js)
    • technically, this is only needed in web at the moment, but it seems likely that we'll hit the same issue in other Angular clients when someone adds a test. I thought I'd just add it now to avoid that issue arising
  • apps/web/src/app/core/index.ts and related imports - remove the old hack

Screenshots

Before you submit

  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

eliykat avatar Dec 14 '22 01:12 eliykat