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

Use full schema object instead of an array for jsx-curly-spacing #1292

Merged
merged 1 commit into from Jul 11, 2017

Conversation

jseminck
Copy link
Contributor

@jseminck jseminck commented Jul 9, 2017

See: #1290

@ljharb ljharb merged commit 75fb917 into jsx-eslint:master Jul 11, 2017
@ljharb
Copy link
Member

ljharb commented Jul 11, 2017

@yannickcr it'd be great to release this as a semver-patch ASAP (assuming nothing minor or major is already in master)

Hypnosphi added a commit to JetBrains/eslint-config that referenced this pull request Jul 17, 2017
BREAKING CHANGE: eslint@4.2.0 or higher is now a peer dependency

Added rules:
 * [`react/no-find-dom-node`](https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/no-find-dom-node.md)
 * [`react/no-redundant-should-component-update`](https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/no-redundant-should-component-update.md)
 * [`getter-return`](http://eslint.org/docs/rules/getter-return)
 * [`multiline-ternary`](http://eslint.org/docs/rules/multiline-ternary)
 * [`object-curly-newline`](http://eslint.org/docs/rules/object-curly-newline)
 * [`semi-style`](http://eslint.org/docs/rules/semi-style)
 * [`switch-colon-spacing`](http://eslint.org/docs/rules/switch-colon-spacing)

Added options:
 *
 * `{"ignoreComments": true}` for [`no-trailing-spaces`](http://eslint.org/docs/rules/no-trailing-spaces)
 * `{"newlines-between": "always-and-inside-groups"}` for [`import/order`](https://github.com/benmosher/eslint-plugin-import/blob/HEAD/docs/rules/order.md)

[`react/jsx-curly-spacing`](https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-curly-spacing.md) is temporarily disabled until a [fix](jsx-eslint/eslint-plugin-react#1292) gets published
@ZebraFlesh
Copy link

Is there something which is blocking publishing a release with this PR? Not having it is blocking upgrades from eslint 4.1.1 to 4.2.0.

@ljharb
Copy link
Member

ljharb commented Jul 18, 2017

@ZebraFlesh in fact this PR is not necessary for upgrading to eslint 4.2.0; the v4.2.0 upgrade is necessary to avoid an error. This PR enforces the schema - without this PR, invalid configs pass.

@lukeapage
Copy link
Contributor

@ljharb is it that 4.2 doesn't bail with an error but still logs an error message? That's what I experience and why I've decided to wait for this to be published before upgrading.

@ljharb
Copy link
Member

ljharb commented Jul 18, 2017

@lukeapage yes, exactly - it logs an error message (which is easily ignored) but no longer fails. You're welcome to wait, but it's not really "blocking", it's just slightly inconvenient :-)

@ZebraFlesh
Copy link

That's exactly what I'm talking about: I don't want to try and explain to everyone why there are new errors in the logs and why they're okay.

@Djspaceg
Copy link

The error you refer to @ljharb, is manifested as a dialog box when running eslint in Sublime Text. Which is less easy to ignore. You're even worse-off if you have your files be auto-linted on save, and worse still when you have your files auto-saved when the document loses focus (switching tabs, documents, apps, etc). So literally every time I switch windows, change documents, or anything meaningful, I see this warning dialog box. I'm not saying this is a majority case at all, I'm just saying that there are other cases where a warning isn't as easy to ignore as it is for other people. I would welcome a hotfix to resolve this issue.

@flamerohr
Copy link

error messages shouldn't be considered "ok" or "can be ignored", that's what warning messages are for
it would be good to get a resolution for this, rather than have these warnings show up

@ljharb
Copy link
Member

ljharb commented Jul 21, 2017

This is a warning message (from eslint 4), not an error message.

@ahmadnassri
Copy link

can we please get a ping on this thread, when the next release goes out with this fix?

@msikma
Copy link

msikma commented Jul 28, 2017

If, like me, you're annoyed by the error message, my recommendation would just be to replace your old file with the patched one manually for now.

Quick way to do it (from your project directory):

curl -o ./node_modules/eslint-plugin-react/lib/rules/jsx-curly-spacing.js "https://raw.githubusercontent.com/jseminck/eslint-plugin-react/75fb917c1ce27540f48cbe3fa7436a6822d4b953/lib/rules/jsx-curly-spacing.js"

The new release will come around soon enough, assuming you don't clear out your modules too often this will clear up your terminal a bit until the next version rolls around. 🙂

screen shot 2017-07-29 at 00 58 14

@almirfilho
Copy link

Could you guys bump a new version with this, please?
Thank you very much! =)

wKovacs64 added a commit to wKovacs64/hibp that referenced this pull request Aug 4, 2017
N.B. The "can't resolve reference" warnings are coming from
eslint-plugin-react and can be safely ignored until a version of
eslint-plugin-react containing PR #1292 is released.

See:
airbnb/javascript#1488
jsx-eslint/eslint-plugin-react#1292
dainyl added a commit to nwaywood/react-fullstack-template that referenced this pull request Aug 8, 2017
N.B. The "can't resolve reference" warnings are coming from
eslint-plugin-react and can be safely ignored until a version of
eslint-plugin-react containing PR #1292 is released.

See:
airbnb/javascript#1488
jsx-eslint/eslint-plugin-react#1292
@antoinerousseau
Copy link

Please publish this on NPM asap.

@yannickcr
Copy link
Member

Released in v7.2.0, really sorry for the delay.

@Kerumen
Copy link
Contributor

Kerumen commented Aug 9, 2017

@yannickcr Thank you!

@Gabri3l
Copy link

Gabri3l commented Aug 17, 2017

Thank you for the release!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet