-
Notifications
You must be signed in to change notification settings - Fork 180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: update typescript to 5.3.3 #14407
Conversation
updates typescript to latest. updates eslint to latest. fixes some typescript errors, ignores some typescript errors. suppresses new eslint errors.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## edge #14407 +/- ##
==========================================
- Coverage 68.25% 68.21% -0.05%
==========================================
Files 1629 2520 +891
Lines 54889 71912 +17023
Branches 4125 9188 +5063
==========================================
+ Hits 37465 49053 +11588
- Misses 16735 20686 +3951
- Partials 689 2173 +1484
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -5,6 +5,7 @@ | |||
**/build/** | |||
**/venv/** | |||
.opentrons_config | |||
**/tsconfig*.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typescript-eslint/parser seems unable to parse tsconfig json files, so we're ignoring them
'standard-with-typescript', | ||
'plugin:react/recommended', | ||
'prettier', | ||
'plugin:json/recommended', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
turns out, without extending these rules we weren't actually linting json files in edge. so now we're actually linting json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
downside: linting json via typescript-eslint/parser takes about an extra minute to run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol fyi @sfoster1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
twas a breaking change in a previous version of https://github.com/azeemba/eslint-plugin-json?tab=readme-ov-file#eslint-plugin-json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. smoke tested PD and stuff still seems to work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woo, thank you Brent! I mainly tested PD and LL. Looks good to me!
@@ -13,7 +13,7 @@ const page: Reducer<Page, any> = handleActions( | |||
}, | |||
'file-splash' | |||
) | |||
// @ts-expect-error(sa, 2021-6-21): TS thinks this will only return false, even though TOGGLE_NEW_PROTOCOL_MODAL might yield true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
schweet
Overview
updates typescript to latest. updates eslint to latest. fixes some typescript errors, ignores some typescript errors. suppresses new eslint errors. enables json linting, fixes json linting errors.
Test Plan
CI passing, builds building
Changelog
Review requests
check local builds
Risk assessment
medium-high