Skip to content

Fix 18 JS type errors for TSC to output 683 TS/TSX errors#5975

Merged
leotm merged 4 commits intomainfrom
fix/js-static-type-errors
Mar 22, 2023
Merged

Fix 18 JS type errors for TSC to output 683 TS/TSX errors#5975
leotm merged 4 commits intomainfrom
fix/js-static-type-errors

Conversation

@leotm
Copy link
Copy Markdown
Contributor

@leotm leotm commented Mar 16, 2023 •
edited
Loading

Development & PR Process

  1. Follow MetaMask Mobile Coding Standards
  2. Add release-xx label to identify the PR slated for a upcoming release (will be used in release discussion)
  3. Add needs-dev-review label when work is completed
  4. Add needs-qa label when dev review is completed
  5. Add QA Passed label when QA has signed off

Description

Minimum required to see/tackle remaining 683 errors in 120 files in our TSC

Follow-up to: #5879
Fix: #5878

After this PR merge #5882 to reduce to 556 errors in 87 files

Then i can begin refactoring to TS (in similar manner to @gantunesr's doing in MetaMask/KeyringController#202)

cc @Cal-L

Screenshots/Recordings

In aforementioned links above ^

Issue

Progresses #5878

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@leotm
Fix 18 JS type errors exposing 683 TS/TSX errors
dae1e1d 
Minimum required to tackle remaining errors in 120 files

Follow-up to: #5879

Fix: #5878

Next: #5882
@leotm leotm requested a review from a team as a code owner March 16, 2023 14:24
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 16, 2023

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@leotm leotm mentioned this pull request Mar 16, 2023
Closed
3 tasks
@leotm leotm added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) No QA Needed Apply this label when your PR does not need any QA effort. labels Mar 16, 2023
@leotm leotm changed the title Fix 18 JS type errors exposing 683 TS/TSX errors Fix 18 JS type errors exposing 683 TS/TSX errors in TSC Mar 16, 2023
@leotm leotm changed the title Fix 18 JS type errors exposing 683 TS/TSX errors in TSC Fix 18 JS type errors exposing 683 TS/TSX errors via TSC Mar 16, 2023
@leotm leotm mentioned this pull request Mar 16, 2023
Closed
@leotm leotm changed the title Fix 18 JS type errors exposing 683 TS/TSX errors via TSC Fix 18 JS type errors for TSC to output 683 TS/TSX errors Mar 17, 2023
leotm
leotm commented Mar 17, 2023
View reviewed changes
leotm
leotm commented Mar 17, 2023
View reviewed changes
@Cal-L
Copy link
Copy Markdown
Contributor

Cal-L commented Mar 17, 2023

After running yarn tsc, I'm seeing a bunch of errors from node_modules and from the app. Any ideas what I might be doing wrong?

@leotm
Copy link
Copy Markdown
Contributor Author

leotm commented Mar 18, 2023

After running yarn tsc, I'm seeing a bunch of errors from node_modules and from the app. Any ideas what I might be doing wrong?

yep that's expected ^ so this pr fixes our

which enables TSC to emit our 683 TS/TSX type errors lurking behind 😅

so next step is to tackle these ^ just created an issue for us here

@leotm
Copy link
Copy Markdown
Contributor Author

leotm commented Mar 18, 2023 •
edited
Loading

after merging current pr, this pr below reduces the node_modules errors by a lot

so think you're seeing the screenshots in the issue

but i can't seem to figure why the last few node_modules errors remain there 🤔 (stemming from 2 libs)

Cal-L
Cal-L approved these changes Mar 20, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a small comment. LGTM

gantunesr
gantunesr approved these changes Mar 22, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@gantunesr gantunesr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@leotm leotm merged commit d3f01d4 into main Mar 22, 2023
@leotm leotm deleted the fix/js-static-type-errors branch March 22, 2023 15:48
@github-actions github-actions bot locked and limited conversation to collaborators Mar 22, 2023
@leotm leotm added the type-tech-debt Technical debt label Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Cal-L Cal-L Cal-L approved these changes

@gantunesr gantunesr gantunesr approved these changes

Assignees

No one assigned

Labels

needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) No QA Needed Apply this label when your PR does not need any QA effort. type-tech-debt Technical debt

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

JavaScript static type errors

3 participants

@leotm @Cal-L @gantunesr