fix: Upgrade typescript configuration #91
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #91 +/- ##
=======================================
Coverage 99.21% 99.21%
=======================================
Files 14 14
Lines 509 509
Branches 180 180
=======================================
Hits 505 505
Misses 4 4 ☔ View full report in Codecov by Sentry. |
just-boris
force-pushed
the
es-target
branch
from
a75e648 to
56dbff5
Compare
just-boris
force-pushed
the
es-target
branch
from
56dbff5 to
d93c886
Compare
| @@ -12,6 +12,7 @@ | |||
| "test-08:00": "TZ=America/Los_Angeles vitest --run ./src/__tests__/date-utils --coverage=false", | |||
| "test+08:00": "TZ=Asia/Brunei vitest --run ./src/__tests__/date-utils --coverage=false", | |||
| "test-local": "vitest --run", | |||
| "pretest": "tsc -p tsconfig.unit.json", | |||
There was a problem hiding this comment.
Regression happened when migrating to vitest: #73
Unlike ts-jest, vitest does not do type checks when running. Added this step and fixed a bunch of type errors existed in the project
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "name": "@cloudscape-design/collection-hooks", | |||
| "version": "1.0.0", | |||
| "lockfileVersion": 2, | |||
| "lockfileVersion": 3, | |||
There was a problem hiding this comment.
Since we use node.js 20, the lockfile must be version 3
| @@ -58,12 +59,13 @@ | |||
| "eslint-plugin-react-hooks": "^4.3.0", | |||
| "eslint-plugin-unicorn": "^11.0.2", | |||
| "husky": "^9.0.0", | |||
| "jsdom": "^25.0.1", | |||
There was a problem hiding this comment.
Unlike Jest, in Vitest there is no special "environment" package, but it expects JSDOM to be installed as a dependency. Before this change, it relied on the implicit version appeared in package-lock file, now it is explicit
| "target": "es5", | ||
| "target": "es2015", | ||
| "module": "esnext", | ||
| "declaration": true, | ||
| "types": [], | ||
| "esModuleInterop": true, | ||
| "moduleResolution": "Node", | ||
| "lib": ["es5", "es2015.collection"], | ||
| "lib": ["es2020"], |
There was a problem hiding this comment.
Pulling changes from the main components config
- Bump target and lib to es2015/es2020 respectively
- Set
types: []. The typescript default is "load everything from thenode_modules/@typesdirectory" which we do not want. We only want the types listed in thelibsection
| * subset of node.js types, safe to use in browser and bundlers | ||
| */ | ||
| declare const process: { env: { NODE_ENV?: string } }; | ||
| declare const console: { warn: (...args: Array<any>) => void }; |
There was a problem hiding this comment.
Since we do not load neither @types/node or dom in our code, need to provide shims for this API.
(Fun fact, console object is not a part of Javascript spec, it is separately described in Node and DOM specs)
There was a problem hiding this comment.
Which types?
dom types would undermine SSR support, require to write more compatibility tests
@types/node would undermine browser support, potentially causing issues when bundling
so, let's keep it a narrow manually-managed list. I do not expect this list to grow more than that (only setTimeout comes to mind as another "node or dom" type we might need)
There was a problem hiding this comment.
Got it, thanks. I think the SSR support reasoning can be added as a code comment, too.
| @@ -1,7 +1,8 @@ | |||
| { | |||
| "extends": "./tsconfig.json", | |||
| "compilerOptions": { | |||
| "lib": ["es2019", "dom"], | |||
| "lib": ["es2020", "dom"], | |||
| "moduleResolution": "bundler", | |||
There was a problem hiding this comment.
There is an issue when running type checks for test files. Vitest does not pass type checks unless moduleResolution: bundler is used: vitest-dev/vitest#4567
Needed to upgrade typescript for this as well
| * subset of node.js types, safe to use in browser and bundlers | ||
| */ | ||
| declare const process: { env: { NODE_ENV?: string } }; | ||
| declare const console: { warn: (...args: Array<any>) => void }; |
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
Issue #, if available:
Notes
Align configuration with cloudscape-design/components and fix some build issues
Testing
Dry-runs: 7169913711, 7169914866
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.