Skip to content
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

style(app, protocol-designer, components): promote no-unsafe-argument from warn to error, fix all #15402

Merged
merged 29 commits into from
Jun 20, 2024

Conversation

b-cooper
Copy link
Contributor

Overview

Promote the typescript eslint rule for no-unsafe-argument to error and fix all outstanding instances
in the mono repo

Review requests

  • smoke test front end apps

Risk assessment

medium, should have no effect on code, but many files were touched

@b-cooper b-cooper requested a review from shlokamin June 12, 2024 20:40
@b-cooper b-cooper requested review from a team as code owners June 12, 2024 20:40
Comment on lines +11 to +14
properties: Record<string, unknown>
superProperties?: Record<string, unknown>
}
| { superProperties: { [key: string]: unknown } }
| { superProperties: Record<string, unknown> }
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this error with the new lint error or is this a random refactor? (asking because i 100% prefer it written this way).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a slightly unrelated fix, and probably not necessary here. I'm actually in favor of the index-signature syntax of the Record keyword myself. I think we probably want to relax or invert that typescript eslint rule. I'll tackle that in the next PR.

Copy link
Collaborator

@jerader jerader left a comment

Choose a reason for hiding this comment

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

this is very nice, thank you! 📝 🦖 . i smoke tested PD primarily and bopped around a bit in the app (nevermind i thought i did but it actually whitescreens) but PD looks good to me

@jerader
Copy link
Collaborator

jerader commented Jun 17, 2024

this is the app whitescreen error i am getting

Screenshot 2024-06-17 at 10 25 35

@b-cooper b-cooper merged commit 0c54e07 into edge Jun 20, 2024
68 checks passed
@b-cooper b-cooper deleted the chore_no-unsafe-argument branch June 20, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants