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
Contributor

jseminck commented Jul 9, 2017

See: #1290

@ljharb

ljharb approved these changes Jul 11, 2017

@ljharb ljharb merged commit 75fb917 into yannickcr:master Jul 11, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 96.047%
Details
@ljharb

This comment has been minimized.

Collaborator

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

[Publish] feat: Update packages, add eslint peer dependency
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](yannickcr/eslint-plugin-react#1292) gets published
@ZebraFlesh

This comment has been minimized.

ZebraFlesh commented Jul 18, 2017

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

This comment has been minimized.

Collaborator

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

This comment has been minimized.

Contributor

lukeapage commented Jul 18, 2017

@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

This comment has been minimized.

Collaborator

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

This comment has been minimized.

ZebraFlesh commented Jul 18, 2017

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

This comment has been minimized.

Djspaceg commented Jul 19, 2017

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

This comment has been minimized.

flamerohr commented Jul 21, 2017

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

This comment has been minimized.

Collaborator

ljharb commented Jul 21, 2017

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

@ahmadnassri

This comment has been minimized.

ahmadnassri commented Jul 27, 2017

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

@msikma

This comment has been minimized.

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

This comment has been minimized.

almirfilho commented Aug 2, 2017

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

Upgrade eslint to v4
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
yannickcr/eslint-plugin-react#1292

dainyl added a commit to nwaywood/react-fullstack-template that referenced this pull request Aug 8, 2017

Updated outdated packages
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
yannickcr/eslint-plugin-react#1292
@antoinerousseau

This comment has been minimized.

antoinerousseau commented Aug 8, 2017

Please publish this on NPM asap.

@yannickcr

This comment has been minimized.

Owner

yannickcr commented Aug 9, 2017

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

@Kerumen

This comment has been minimized.

Contributor

Kerumen commented Aug 9, 2017

@yannickcr Thank you!

@Gabri3l

This comment has been minimized.

Gabri3l commented Aug 17, 2017

Thank you for the release!!

@willdurand willdurand referenced this pull request Aug 17, 2017

Merged

Eslint 4.3 #2860

@jseminck jseminck deleted the jseminck:jsx-curly-spacing-schema branch Nov 19, 2017

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