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.

Sign up for GitHub

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

Jump to bottom

v6.7.0 broke autofixing for eslint-plugin-react #12614

Closed
ljharb opened this issue Nov 28, 2019 · 1 comment · Fixed by #12616, basics/vector#113, basics/vector#127, thinkwee/thinkwee.github.io#39 or alxtford/numconv#46
Closed

v6.7.0 broke autofixing for eslint-plugin-react #12614

ljharb opened this issue Nov 28, 2019 · 1 comment · Fixed by #12616, basics/vector#113, basics/vector#127, thinkwee/thinkwee.github.io#39 or alxtford/numconv#46
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly patch candidate This issue may necessitate a patch release in the next few days

Comments

@ljharb
Copy link
Contributor

ljharb commented Nov 28, 2019

Tests pass on master in https://github.com/yannickcr/eslint-plugin-react on v6.6, and fail on v6.7.0 or v6.7.1. https://travis-ci.org/yannickcr/eslint-plugin-react/jobs/616996307 is an example of the failures; they're related to whitespace placement, within jsx.

I'm not sure which item in the changelog might be responsible, but perhaps #12491 (cc @kaicataldo)?

The change doesn't need to be reverted if i have a reliable way in the react plugin to detect the different behavior, but I suspect that's also difficult.
@ljharb ljharb added bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Nov 28, 2019
@ljharb ljharb mentioned this issue Nov 28, 2019
Merged
ljharb added a commit to jsx-eslint/eslint-plugin-react that referenced this issue Nov 28, 2019
@ljharb
[Tests] temporarily pin eslint 6 tests to v6.6
66aff76 
Pending eslint/eslint#12614
ljharb added a commit to jsx-eslint/eslint-plugin-react that referenced this issue Nov 28, 2019
@ljharb
[Tests] temporarily pin eslint 6 tests to v6.6
71c7d01 
Pending eslint/eslint#12614
@mysticatea mysticatea added patch candidate This issue may necessitate a patch release in the next few days accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Nov 28, 2019
@mysticatea
Copy link
Member

Thank you for your report.

I confirmed it.
As you mentioned, #12491 has changed the behavior of the isSpaceBetweenTokens() method about JSXText tokens that contain only spaces.

mysticatea added a commit that referenced this issue Nov 28, 2019
@mysticatea
Fix: isSpaceBetween should recognize the spaces in JSXText (fixes #12614
e6de965 
)
@mysticatea mysticatea mentioned this issue Nov 28, 2019
Merged
mysticatea added a commit that referenced this issue Nov 28, 2019
@mysticatea
Fix: isSpaceBetween() recognizes spaces in JSXText (fixes #12614)
8e73ee3
kaicataldo pushed a commit that referenced this issue Nov 30, 2019
@mysticatea @kaicataldo
Fix: isSpaceBetweenTokens() recognizes spaces in JSXText (fixes #12614)…
bc435a9 
… (#12616)

* Fix: isSpaceBetween() recognizes spaces in JSXText (fixes #12614)

* apply this fix only for isSpaceBetweenTokens()

* move tests to the section for nodes

* add tests for isSpaceBetween()

* add tests for reversed order
@mysticatea mysticatea mentioned this issue Nov 30, 2019
Closed
ljharb added a commit to ljharb/eslint-plugin-react that referenced this issue Nov 30, 2019
@ljharb
Revert "[Tests] temporarily pin eslint 6 tests to v6.6"
1918430 
This reverts commit 71c7d01.

Thanks to v6.7.2 / eslint/eslint#12616 / eslint/eslint#12614
@snyk-bot snyk-bot mentioned this issue Dec 11, 2019
Merged
This was referenced Jan 12, 2020
Closed
Merged
@snyk-bot snyk-bot mentioned this issue Feb 13, 2020
Merged
@snyk-bot snyk-bot mentioned this issue Feb 27, 2020
Merged
@a-martynovich a-martynovich mentioned this issue Mar 10, 2020
Merged
This was referenced Mar 11, 2020
Merged
Open
This was referenced Mar 26, 2020
Merged
Merged
Merged
Open
This was referenced Apr 3, 2020
Merged
Open
@snyk-bot snyk-bot mentioned this issue Apr 4, 2020
Merged
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators May 30, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label May 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly patch candidate This issue may necessitate a patch release in the next few days
Projects
None yet
Milestone
No milestone
2 participants
@ljharb @mysticatea