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

refactor: add no-unsafe-call lint and improve types #461

Merged
merged 5 commits into from
Feb 7, 2024

Conversation

LightDDark
Copy link
Collaborator

@LightDDark LightDDark commented Feb 7, 2024

Description

fixed linting issues after removing:
'@typescript-eslint/no-unsafe-call': 'off'
issue #450

What I did:

  • Changed setupProxy.js to be ts file and added types
  • Added types to operator.ts
  • Disabled eslint for complaint.js (changing it to ts file is causing check errors in PR)
  • Added eslint-disable-next-line @typescript-eslint/no-unsafe-call for two lines in the utils.ts file due to special cases.

There might be a need to change the config to exempt js files from eslint
@NoamGaash

screenshots

image
image
image

Copy link
Member

@NoamGaash NoamGaash left a comment

Choose a reason for hiding this comment

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

Great PR, thank you!
You can fix the suggestion, or alternatively merge it as-is

})

const res = relevant.reduce((acc: Operator[], name: string): Operator[] => {
return agencyMap.has(name)
? [...acc, { name, id: agencyMap.get(name)?.operator_ref.toString() }]
? [...acc, { name, id: (agencyMap.get(name) as Agency).operator_ref.toString() }]
Copy link
Member

Choose a reason for hiding this comment

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

is as Agency necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we leave it as is it won't be assignable:

Type '{ name: string; id: string | undefined; }[]' is not assignable to type 'Operator[]'.
  Type '{ name: string; id: string | undefined; }' is not assignable to type 'Operator'.
    Types of property 'id' are incompatible.
      Type 'string | undefined' is not assignable to type 'string'.
        Type 'undefined' is not assignable to type 'string'.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
? [...acc, { name, id: (agencyMap.get(name) as Agency).operator_ref.toString() }]
? [...acc, { name, id: (agencyMap.get(name)!).operator_ref.toString() }]

will tell it that it can't be undefined

src/setupProxy.ts Outdated Show resolved Hide resolved
@NoamGaash NoamGaash changed the title chore: Fix linting issues by adding types and disabling rule for spec… refactor: add no-unsafe-call lint and improve types Feb 7, 2024
Co-authored-by: Noam Gaash <noam.gaash@gmail.com>
@LightDDark LightDDark enabled auto-merge (squash) February 7, 2024 16:23
@LightDDark
Copy link
Collaborator Author

Thank you !

@LightDDark LightDDark merged commit 7860cad into hasadna:main Feb 7, 2024
16 checks passed
@LightDDark LightDDark self-assigned this Feb 7, 2024
@LightDDark LightDDark deleted the solve-unsafe-call-lint branch April 3, 2024 09:22
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