Update code to match internal Embrace repo #11
Conversation
gabebw-grammarly
force-pushed
the
update-embrace-v0.1.51
branch
from
7d0fd8e
to
ce8de80
Compare
* Update to version 0.1.51 (the latest internal version)
* Move `tests/` to `src/__tests__`/ to match internal code
* Do not build tests when doing `yarn build`
* This meant excluding the spec files from `tsconfig.json`, so this
commit also introduces `tsconfig.eslint.json` so that ESLint will
still find and lint those test files.
* Update `jest.config.js` for new version of ts-jest
* Update package versions
gabebw-grammarly
force-pushed
the
update-embrace-v0.1.51
branch
from
ce8de80
to
54d34f7
Compare
| @@ -2,6 +2,7 @@ | |||
| "compilerOptions": { | |||
| "declaration": true, | |||
| "declarationMap": true, | |||
| "importHelpers": true, | |||
There was a problem hiding this comment.
I changed very little here. This is not a perfect match for our internal tsconfig.json (or our internal tools/tsconfig.base.json), but most of the other settings don't affect our code.
I decided to keep importHelpers: true, since it helps with bundle size and performance according to https://www.npmjs.com/package/tslib:
Because this can avoid duplicate declarations of things like __extends, __assign, etc., this means delivering users smaller files on average, as well as less runtime overhead. For optimized bundles with TypeScript, you should absolutely consider using
tsliband--importHelpers.
We are already using tslib, so why not?
Jest v29 pulls in pretty-format, which does not work with Node 12, as
shown in [this test run][1]:
Run yarn install
yarn install v1.22.21
info No lockfile found.
[1/4] Resolving packages...
warning @grammarly/tslint-config > [email protected]: TSLint has been deprecated in favor of ESLint. Please see palantir/tslint#4534 for more information.
warning jest-environment-jsdom > jsdom > [email protected]: Use your platform's native atob() and btoa() methods instead
warning jest-environment-jsdom > jsdom > [email protected]: Use your platform's native DOMException instead
warning jest-environment-jsdom > jsdom > data-urls > [email protected]: Use your platform's native atob() and btoa() methods instead
warning [email protected]: Package no longer supported. Consider using eslint-plugin-sonarjs.
[2/4] Fetching packages...
error [email protected]: The engine "node" is incompatible with this module. Expected version "^14.15.0 || ^16.10.0 || >=18.0.0". Got "12.22.12"
error Found incompatible module.
By testing against newer Node versions, we avoid this error.
Also, in general it's a good idea: Node 12 is no longer supported.
According to the chart on https://nodejs.org/en/about/previous-releases,
Node 16 is barely supported, and Node 18 and 20 are the stable versions.
[1]: https://github.com/grammarly/embrace/actions/runs/7065183069/job/19234690133?pr=11
| '^.+\\.tsx?$': [ | ||
| 'ts-jest', | ||
| { | ||
| tsconfig: './tsconfig.json', |
There was a problem hiding this comment.
Should this now be tsconfig.eslint.json, so that it does not exclude test files? I don't know. It passes either way.
There was a problem hiding this comment.
the tests are passing, so let's keep the base one. It will be confusing to use eslint config in the jest setup
tests/tosrc/__tests__/ to match internal codeyarn buildtsconfig.json, so this commit also introducestsconfig.eslint.jsonso that ESLint will still find and lint those test files.jest.config.jsfor new version of ts-jestThese files are copied over from our internal repo unchanged:
README.md(well, almost unchanged: I added# Embraceat the top)src/Workflow changes
I had to update our GitHub test workflow from Node 12 since it's no longer supported and throws errors. We now test against modern Node versions (16, 18, and 20). More details are in the commit message: 92cbb84