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

Update eslintrc for TypeScript #9

Merged
merged 8 commits into from
Aug 27, 2021
Merged

Conversation

zachmargolis
Copy link
Contributor

I had an issue in a562b69 where I was shadowing the h function, which ended up breaking JSX parsing

So then I realized there should be a lint for shadowing variables like that... so this is my attempt to try to get eslint wired to catch that.

Current problem is a bunch of "defined but never used" errors that are not correct....

identity-reporting/src/daily-auths-report.tsx
  2:10  error  'h' is defined but never used  no-unused-vars

identity-reporting/src/observablehq.d.ts
  2:12  error  'plot' is defined but never used   no-unused-vars
  2:17  error  'args' is defined but never used   no-unused-vars
  4:12  error  'ruleY' is defined but never used  no-unused-vars
  4:18  error  'args' is defined but never used   no-unused-vars
  5:12  error  'barY' is defined but never used   no-unused-vars
  5:17  error  'data' is defined but never used   no-unused-vars
  5:28  error  'args' is defined but never used   no-unused-vars

identity-reporting/src/report-filter-controls.tsx
  1:25  error  'h' is defined but never used  no-unused-vars

identity-reporting/src/routes/index.tsx
  1:10  error  'h' is defined but never used  no-unused-vars

identity-reporting/src/routes/report.tsx
  1:17  error  'h' is defined but never used  no-unused-vars

identity-reporting/src/table.tsx
   1:10  error  'h' is defined but never used  no-unused-vars
  12:22  error  'n' is defined but never used  no-unused-vars

✖ 13 problems (13 errors, 0 warnings)

The one that is tripping me up is the n argument from table.jsx ...

  numberFormatter?: (n: number) => string;

@aduth
Copy link
Member

aduth commented Aug 27, 2021

For 'h' is defined but never used, we can use eslint-plugin-react, which has an override for this:

https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-uses-react.md

Alternatively, it'd be nice if we could move to the automatic runtime. ESBuild's support for it is limited at best, currently recommended to use the inject option, only available for build. Vite has decent support for it through the jsxInject option, though it's another one of those things where recreating it in the tests is our responsibility.

tl;dr, if we want auto-import:

  1. https://github.com/aduth/liftsheet/blob/c203ec1/vite.config.ts#L10
  2. https://github.com/aduth/liftsheet/blob/c203ec1/mocha.setup.ts#L7

@aduth aduth mentioned this pull request Aug 27, 2021
@aduth
Copy link
Member

aduth commented Aug 27, 2021

For the "no unused vars" in the type definitions, looks to be a combination of:

  • The methods may be expected to be exported
  • The @typescript-eslint/eslint-plugin includes a "replacement" no-unused-vars to account for unused vars in type declaration files.

@zachmargolis
Copy link
Contributor Author

Thanks for helping with these!

@zachmargolis zachmargolis merged commit 897ae1a into try-convert-tsx Aug 27, 2021
aduth added a commit that referenced this pull request Aug 27, 2021
* Convert DailyAuthsReport to TypeScript (TSX)

* Merge imports

* Export ProcessedResult

* Restore inline comment

* Convert report.js, report.test.js to TS

* Convert report-filter-controls to TSX

* Remove a stray $

* Convert table to TSX

- Also move numberFormatter outside of TableData

* Convert routes/index, routes/report to TSX

* Fix issue due to shadowing h function

* Fix extension

* Update eslintrc for TypeScript (#9)

* Update eslintrc for TypeScript
* Move custom typings to typings directory
* Add global type for import.meta
* Use typescript-eslint/eslint-plugin with recommended config
* Auto-import h
* Use ext option for ESLint extensions
https://eslint.org/docs/user-guide/command-line-interface#--ext
* Fix generate-routes

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

* Inject Fragment

* Fix default= to selected=

* More TypeScript

* ReportRoute props types

* Add return type for path function

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov>
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
@zachmargolis zachmargolis deleted the margolis-tsx-eslint branch September 15, 2021 17:27
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