feat: compile as ES Module #431

boxmein · 2023-01-24T15:21:48Z

Description of the change

Set package.json `"type": "module"`

This opts in all JS files in our library to Node.js ES Module mode
This allows us to transpile to JavaScript that utilizes Node.js's ES Modules support

Set tsconfig.json `"module": "nodenext"`

Build output will now use import and export statements, adding new interoperability considerations
This means you need Node.js 14 or above to import the Javascript API
This unlocks Webpack tree-shaking optimizations, and removes some webpack optimization warnings in our own code
This also adds a requirement to specify every file's file extension in the build output, causing issues for Jest and ESLint

Configure eslint to operate in `ES Modules` context

Updated configuration will fix eslint import plugin, so that it works after extensions are specified

Configure Jest to operate in ES Modules mode

ts-jest-resolver will provide proper filename lookup, so that it works after extensions are specified

Update tests

cross-fetch works better than isomorphic-fetch in ES Modules mode, so using that now
isomorphic-ws had no alternative that worked correctly in ES Modules mode... after trying multiple different approaches to work around it, I opted to only run WebSocket tests on Node.

Type of change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactor (Changed implementation, but existing functionality and APIs are not changed)

Related issues

Checklists

Development

Lint rules pass locally
The code changed/added as part of this pull request has been covered with tests
All tests related to the changed code pass in development

Code review

Changes have been reviewed by at least one other engineer

boxmein · 2023-01-25T08:14:33Z

If your project has type: module then import statements need to have extensions. TypeScript will not require extensions on your import statements, nor will it add them for you. Instead they tell you that you have to add a ".js" extension to import a ".ts" file.

TypeScript and <script type="module"></script> microsoft/TypeScript#13422 (comment)
TypeScript and <script type="module"></script> microsoft/TypeScript#13422 (comment)
Provide a way to add the '.js' file extension to the end of module specifiers microsoft/TypeScript#16577 (comment)

Either we:

✅ Add .js to the end of the source code's import statements (and configure our editor/linter to enforce it)
Add .js to the end of the build output's import statements via some tool
Ask node users to run with --es-module-specifier-resolution=node

boxmein · 2023-01-25T11:23:33Z

For "Add .js to the end of the source code's import statements (and configure our editor/linter to enforce it)" we have to switch eslint to eslint-import-resolver-typescript

…pt/issues/16577\#issuecomment-754941937

boxmein · 2023-01-25T12:34:20Z

I also checked if it is possible to go Full ESM -- and use import directly in the browser. The answer is "yes, if RxJS and GraphQL add .js extension to their import statements."

example.html.txt

… version, due to yarn patches)

After struggling for a long while with trying to "shim" isomorphic-ws to work normally in a jest environment, it is easier to just run this test only in a Node environment. Here are the issues why this test fails in JSDOM: 1. Imports fail The statement `import WebSocket from 'isomorphic-ws';` is passed to the Jest module resolver, which utilises `ts-jest-resolver` (in our case) to find the right file to load. Sadly, it will always load the "main" module of a node package. 2. isomorphic-ws "browser" option is ESM for some reason If we use jest.config.cjs "resolver" field [1] to define a custom script, we will be able to modify the "main" field by patching package.json in memory. This works fine, however isomorphic-ws's browser version is an ES Module for some reason, even if the package.json > "type" is "commonjs". 3. Something inside jest doesn't want me to hack it to support ESM If I override the package.json even harder to also change the "type" to "module" just for isomorphic-ws, Jest doesn't respect it 4. ts-jest doesn't want to change the ESM imports to CJS imports Since... we told it to not change ESM imports to CJS imports, it will not. [1]: https://jestjs.io/docs/configuration#resolver-string

boxmein · 2023-01-31T10:16:14Z

Stopped trying to make the subscriptions tests work across two different jest environments.

Explanation here db48074

codecov-commenter · 2023-02-01T13:50:56Z

Codecov Report

Base: 74.66% // Head: 75.96% // Increases project coverage by +1.29% 🎉

Coverage data is based on head (7c179af) compared to base (1484a11).
Patch coverage: 100.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #431      +/-   ##
==========================================
+ Coverage   74.66%   75.96%   +1.29%     
==========================================
  Files          12       12              
  Lines         671      674       +3     
  Branches      137      136       -1     
==========================================
+ Hits          501      512      +11     
+ Misses        169      153      -16     
- Partials        1        9       +8

Impacted Files	Coverage Δ
src/api-base.ts	`92.30% <100.00%> (-0.12%)`	⬇️
src/qminder-api.ts	`100.00% <100.00%> (ø)`
src/services/DeviceService.ts	`35.00% <100.00%> (ø)`
src/services/GraphQLService.ts	`69.46% <100.00%> (+5.00%)`	⬆️
src/services/LineService.ts	`64.51% <100.00%> (ø)`
src/services/LocationService.ts	`94.44% <100.00%> (ø)`
src/services/TicketService.ts	`78.86% <100.00%> (+0.26%)`	⬆️
src/services/UserService.ts	`55.26% <100.00%> (ø)`
src/services/WebhooksService.ts	`100.00% <100.00%> (ø)`
... and 4 more

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

boxmein · 2023-02-01T15:54:02Z

Demonstrating what it does to compilation output

Before this branch:

After this branch:

2ndalpha · 2023-02-01T17:35:25Z

@boxmein should se still test on different NodeJS versions? Currently we have test matrix for v12, 14, 15 & 16

2ndalpha

🚀

boxmein added 6 commits January 24, 2023 17:21

feat: compile as ES Module

2228ba0

update CI workflows to newer node

b9a57ba

release workflow to newer node

2f99559

update yarn version to 3.3.1

47a8f61

explicitly set yarnPath

7b7483c

fix: frozen-lockfile to immutable

ab06e2d

boxmein added 3 commits January 25, 2023 10:43

replace isomorphic-fetch with cross-fetch

b668b13

fix: done() called only once

0ac25f2

update jest + fix a few tests

29ebb78

boxmein added 3 commits January 25, 2023 13:42

Add .js to local imports as per https://github.com/microsoft/TypeScri…

afe68eb

…pt/issues/16577\#issuecomment-754941937

prettier write

c94c038

example for adding it in the browser

b0b4dee

boxmein and others added 7 commits January 25, 2023 14:37

github workflows to one version (hashes may change when changing node…

a2e9b0d

… version, due to yarn patches)

Merge branch 'master' into johannes_esm

6c5e015

update package.json

996dfc6

Update yarn lock

40b187e

fix: graphql subscription tests don't leak handles

d8650ff

Node tests for graphql subscriptions are now working

a1fc733

boxmein added 2 commits February 1, 2023 15:38

prettier write

afd808b

follow flags in tsconfig

7c8ffbc

Merge branch 'master' into johannes_esm

7c179af

boxmein requested review from KaarlisCaune, 2ndalpha and KarlMae February 1, 2023 15:53

2ndalpha approved these changes Feb 3, 2023

View reviewed changes

2ndalpha merged commit 9a6f1b1 into master Feb 3, 2023

2ndalpha deleted the johannes_esm branch February 3, 2023 12:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: compile as ES Module #431

feat: compile as ES Module #431

boxmein commented Jan 24, 2023 •

edited

Loading

boxmein commented Jan 25, 2023 •

edited

Loading

boxmein commented Jan 25, 2023

boxmein commented Jan 25, 2023

boxmein commented Jan 31, 2023

codecov-commenter commented Feb 1, 2023 •

edited

Loading

boxmein commented Feb 1, 2023

2ndalpha commented Feb 1, 2023

2ndalpha left a comment

feat: compile as ES Module #431

feat: compile as ES Module #431

Conversation

boxmein commented Jan 24, 2023 • edited Loading

Description of the change

Set package.json "type": "module"

Set tsconfig.json "module": "nodenext"

Configure eslint to operate in ES Modules context

Configure Jest to operate in ES Modules mode

Update tests

Type of change

Related issues

Checklists

Development

Code review

boxmein commented Jan 25, 2023 • edited Loading

boxmein commented Jan 25, 2023

boxmein commented Jan 25, 2023

boxmein commented Jan 31, 2023

codecov-commenter commented Feb 1, 2023 • edited Loading

Codecov Report

boxmein commented Feb 1, 2023

Demonstrating what it does to compilation output

2ndalpha commented Feb 1, 2023

2ndalpha left a comment

Choose a reason for hiding this comment

boxmein commented Jan 24, 2023 •

edited

Loading

Set package.json `"type": "module"`

Set tsconfig.json `"module": "nodenext"`

Configure eslint to operate in `ES Modules` context

boxmein commented Jan 25, 2023 •

edited

Loading

codecov-commenter commented Feb 1, 2023 •

edited

Loading