search

opengovsg / FormSG

Form builder for the Singapore Government
https://form.gov.sg
Other
260 stars 78 forks source link

fix: resolve eslint warnings on npm builds #7446

Closed g-tejas closed 1 week ago

g-tejas commented 2 weeks ago

Problem

Screenshot 2024-06-26 at 4.39.24 PM.png

This PR resolves the warnings that arise from eslint

Breaking Changes

g-tejas commented 2 weeks ago

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @g-tejas and the rest of your teammates on Graphite Graphite

KenLSM commented 2 weeks ago

Regarding no-unsed-vars 👍

Regarding explicit-module-boundary-types My personal take: for React Components that are not explicitly returning JSX.Element, I feel that it doesn't really add much value to add types to them.

Usually, for these components, we are already using it as a component that will be rendered, and TS is smart enough to know whether the untyped-function is renderable function.

No qualms on adding them, but also not advocating that we should enforce a return to them.

g-tejas commented 2 weeks ago

Regarding no-unsed-vars 👍

Regarding explicit-module-boundary-types My personal take: for React Components that are not explicitly returning JSX.Element, I feel that it doesn't really add much value to add types to them.

Usually, for these components, we are already using it as a component that will be rendered, and TS is smart enough to know whether the untyped-function is renderable function.

No qualms on adding them, but also not advocating that we should enforce a return to them.

Regarding no-unsed-vars 👍

Regarding explicit-module-boundary-types My personal take: for React Components that are not explicitly returning JSX.Element, I feel that it doesn't really add much value to add types to them.

Usually, for these components, we are already using it as a component that will be rendered, and TS is smart enough to know whether the untyped-function is renderable function.

No qualms on adding them, but also not advocating that we should enforce a return to them.

@KenLSM In that case, should we disable explicit-module-boundary-types eslint rule for .tsx files (since these files mostly consist of frontend components? It seems like the majority of the eslint warnings emitted on builds are just this. Either that or we can disable it per file, but that feels weird. Or should we just keep the warnings?

KenLSM commented 2 weeks ago

Sounds good on excluding .tsx files