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

Remove deprecated rule jsx-a11y/href-no-hash for eslint^4.0.0 #2930

Closed
wants to merge 1 commit into from

Conversation

filipposarzana
Copy link

@filipposarzana filipposarzana commented Aug 10, 2017

Remove this rule we can avoid having a lot of warnings during build of CRA to work event with eslint^4.0.0 and eslint-plugin-jsx-a11y^6.0.2

Line 1: Definition for rule 'jsx-a11y/href-no-hash' was not found jsx-a11y/href-no-hash

Remove this rule we can avoid having a lot of warnings during build of CRA

`Line 1:  Definition for rule 'jsx-a11y/href-no-hash' was not found  jsx-a11y/href-no-hash`
@filipposarzana filipposarzana changed the title Remove deprecated rule jsx-a11y/href-no-hash Remove deprecated rule jsx-a11y/href-no-hash for eslint^4.0.0 Aug 10, 2017
@gaearon
Copy link
Contributor

gaearon commented Aug 10, 2017

Why do you have 6.x of this plugin?

We explicitly install "eslint-plugin-jsx-a11y": "5.1.1". So this seems like a package installation problem on your end.

@filipposarzana
Copy link
Author

We have updated eslint-plugin-jsx-a11y^6.0.2 to keep up a11y

they have removed this rule in their next release so i think it's important including it with releases to come

it's the only thing breaking CRA at the moment since my .eslintrc is clean and i have no rules turned of

@gaearon
Copy link
Contributor

gaearon commented Aug 10, 2017

What I'm saying is that the version of the plugin used by CRA is different. How did you get 6.x on your machine? Did you install it manually?

@filipposarzana
Copy link
Author

Yes, of course I installed it manually to keep my project up to date and to have 100% a11y rating.

What I'm suggesting here is to keep CRA up to date too, simply by removing this rule and be prepared to next upgrade of a11y

@gaearon
Copy link
Contributor

gaearon commented Aug 14, 2017

Yes, of course I installed it manually

To be clear this is not currently supported without ejecting. So it's not surprising this caused issues.

Are you sure that just removing it is the right approach? Perhaps there's some other rule that we should enable or make more broad now. In fact I thought we had a PR doing this—did you get a chance to look through existing PRs?

@Timer
Copy link
Contributor

Timer commented Aug 14, 2017

Seems like the rule was replaced by anchor-is-valid which is a more customizable version.

We should probably incorporate it with a decent configuration.

edit: looks as @gaearon has already done this (#2690).

@czebe
Copy link

czebe commented Sep 18, 2017

Any news on this?

@Timer
Copy link
Contributor

Timer commented Sep 18, 2017

There's no problem here, and the plugin doesn't need updated.

It will be updated when we release v2.0.0, however.

@react-scripts-dangerous

Hello! I'm a bot that helps facilitate testing pull requests.

Your pull request (commit 75d380b) has been released on npm for testing purposes.

npm i react-scripts-dangerous@1.0.12-75d380b.0
# or
yarn add react-scripts-dangerous@1.0.12-75d380b.0
# or
create-react-app --scripts-version=react-scripts-dangerous@1.0.12-75d380b.0 folder/

Note that the package has not been reviewed or vetted by the maintainers. Only install it at your own risk!

Thanks for your contribution!

@gaearon
Copy link
Contributor

gaearon commented Jan 12, 2018

Closing in favor of #2690.

@gaearon gaearon closed this Jan 12, 2018
@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants