search

hackforla / lucky-parking

Visualization of parking data to assist in understanding of the effects of parking policies on a neighborhood by neighborhood basis in the City of Los Angeles
https://www.hackforla.org/projects/lucky-parking.html
33 stars 60 forks source link

[Tech Debt] Fix ESLint problems with `react/prop-types` #633

Closed glenflorendo closed 6 months ago

glenflorendo commented 6 months ago

User Story

As a developer, I want to adhere to our linting rules, so that I can ensure I am writing clean, correct code and following best practices.

Description

ESLint is a popular tool that is used to analyze code and find problems based on our configured rules, which includes several nice recommendations.

This ticket focuses on the react/prop-types.

You will need to understand this rule and go through the code to resolve any problems.

To see all violations for all rules, run the following command:

pnpm ci:lint

This rule should be removed from our configuration to allow the default settings to come into affect. It was merely added as an override to not block development.

Additional Context

This work is being done now in part to Ci/CD efforts. We want to enable linting in our pipeline, however, it will always fail since we currently have errors.

Acceptance Criteria

  1. Running the linter will not find any violations with this rule.
  2. This rule is removed from our configuration.

Technical References

glenflorendo commented 6 months ago

@bzzz-coding Let's convert the PropTypes into TypeScript. That should also resolve this issue.

bzzz-coding commented 6 months ago

@glenflorendo I don't see PropTypes being used in our project. Are you okay with turning off the prop-types rule (change react/prop-types: warn to react/prop-types: off)? This rule seems redundant to me since we use TypeScript. When I run pnpm ci:lint, the rule warns about the Loader props missing in props validation, but I have defined the Loader props using TypeScript interfaces, which are used for type checking. What are your thoughts?

glenflorendo commented 6 months ago

@glenflorendo I don't see PropTypes being used in our project. Are you okay with turning off the prop-types rule (change react/prop-types: warn to react/prop-types: off)? This rule seems redundant to me since we use TypeScript. When I run pnpm ci:lint, the rule warns about the Loader props missing in props validation, but I have defined the Loader props using TypeScript interfaces, which are used for type checking. What are your thoughts?

Ah, I see what you mean.

I took a look at the Loader component, and I think it's how we used React.FC here. Instead, let's rewrite the function declaration to how this article does it. I think that should remove this error.

Also, could you move the prop comments to the interface instead?