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

Stricter linter #3095

Merged
merged 9 commits into from
Aug 17, 2021
Merged

Stricter linter #3095

merged 9 commits into from
Aug 17, 2021

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Aug 11, 2021

  • Disabling no-await-in-loop rule as I don't see why we would want to enforce that.
  • Convert rules that have 2 or less warnings to error.
  • Enforce all accessibility rules to error so we don't miss them.

Remaining rules that warn:

Rule Number of remaining warnings
class-methods-use-this 48
consistent-return 85
default-case 8
global-require 105
import/no-unresolved 27
import/order 10
no-mixed-operators 8
no-param-reassign 99
no-redeclare 60
no-shadow 41
no-unused-expressions 7
no-unused-vars 113
no-use-before-define 156
radix 7
react/destructuring-assignment 340
react/jsx-props-no-spreading 36
jsdoc/check-param-names 11
jsdoc/check-syntax 12

@arturi
Copy link
Contributor

arturi commented Aug 11, 2021

Thanks for attacking this!

Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

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

Nice one

@@ -81,6 +81,7 @@ module.exports = function Dashboard (props) {
onDrop={props.handleDrop}
>
<div
aria-hidden="true"
Copy link
Member

Choose a reason for hiding this comment

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

Why was this added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

   83:7   error    Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   83:7   error    Static HTML elements with event handlers require a role                                         jsx-a11y/no-static-element-interactions

Since it's the overlay, I thought the easiest thing to do was to hide it from screen readers, but maybe there's a better way?

@Murderlon
Copy link
Member

Some tests are still failing though

@aduh95 aduh95 merged commit 6b7ad5e into main Aug 17, 2021
@aduh95 aduh95 deleted the stricter-linter branch August 17, 2021 18:32
Murderlon added a commit that referenced this pull request Aug 19, 2021
* 'main' of https://github.com/transloadit/uppy:
  Add referrer to transloadit.com link (#3116)
  @uppy/locales latest version is 1.22.0 🙈
  Stricter linter (#3095)
  @uppy/aws-s3: refactor to use private fields (#3094)
  build: fix legacy bundle (#3112)
  Fix locales — point to CDN v1.31.0
Murderlon added a commit that referenced this pull request Aug 25, 2021
* main: (23 commits)
  Release
  Set node version in `workflows/cdn.yml` to 16.x
  Release
  build: add stylelint (#3124)
  Core: rename allowMultipleUploads to allowMultipleUploadBatches (#3115)
  meta: enforce `no-unused-vars` linter rule (#3118)
  writing-plugins: update example to use `i18nInit` (#3122)
  @uppy/core: reject empty string as valid value for required meta fields (#3119)
  Safely escape <script> injected code in companion `send-token.js` (#3101)
  @uppy/dashboard: fix metafield form validation (#3113)
  Clean up `BACKLOG.md` & add Vimeo as todo
  Add referrer to transloadit.com link (#3116)
  @uppy/locales latest version is 1.22.0 🙈
  Stricter linter (#3095)
  @uppy/aws-s3: refactor to use private fields (#3094)
  build: fix legacy bundle (#3112)
  Fix locales — point to CDN v1.31.0
  Fix typo in `docs/companion.md`
  Changelog for 1.31.0 and patches
  Strictly type uppy events (#3085)
  ...
HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
* enforce some eslint rules

* enforce accessibility linter rules

* harden lint rules with only 1 or 2 warnings

* fix remaining rules with less than 3 warnings

* fix e2e tests

* fix remaining rules with less than 4 warnings

* fix remaining rules with less than 6 warnings

* fix `shuffleTaglines`

* fix companion build
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.

3 participants