Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

feat: add "liferay/metal" preset #50

Merged
merged 1 commit into from
Jul 9, 2019
Merged

feat: add "liferay/metal" preset #50

merged 1 commit into from
Jul 9, 2019

Conversation

wincent
Copy link
Contributor

@wincent wincent commented Jul 9, 2019

In a project like dynamic-data-mapping-form-builder, we use metal-jsx. For reasons described here:

https://eslint.org/blog/2015/03/eslint-0.17.0-released#changes-to-jsxreact-handling

ESLint will spuriously warn that variables that are only used as components inside JSX are not used. We can't just turn on the "liferay/react" preset in these projects because that includes a bunch of rules that don't make sense in a metal-jsx context.

So, we create a metal-specific preset here that contains just one rule:

https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-uses-vars.md

Switching to this preset makes all 23 currently-existing lint errors/warnings in the dynamic-data-mapping-form-builder project go away, as seen in these two before-and-after runs:

https://gist.github.com/wincent/16428f82e4ce74f8f5d6f73271ef1aef

The one remaining problem will be projects that use a mixture of metal-jsx and React (@andresfulla is currently dealing with one of those); we'll have to figure out what to do there along the way.

Test plan: yarn add the package to liferay-portal, create an .eslintrc.js as described in the README, and run yarn checkFormat.

In a project like dynamic-data-mapping-form-builder, we use metal-jsx.
For reasons described here:

https://eslint.org/blog/2015/03/eslint-0.17.0-released#changes-to-jsxreact-handling

ESLint will spuriously warn that variables that are only used as
components inside JSX are not used. We can't just turn on the
"liferay/react" preset in these projects because that includes a bunch
of rules that don't make sense in a metal-jsx context.

So, we create a metal-specific preset here that contains just one rule:

https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-uses-vars.md

Switching to this preset makes all 23 currently-existing lint
errors/warnings in the dynamic-data-mapping-form-builder project go
away, as seen in these two before-and-after runs:

https://gist.github.com/wincent/16428f82e4ce74f8f5d6f73271ef1aef

The one remaining problem will be projects that use a mixture of
metal-jsx and React (@andresfulla is currently dealing with one of
those); we'll have to figure out what to do there along the way.

Test plan: `yarn add` the package to liferay-portal, create an
`.eslintrc.js` as described in the README, and run `yarn checkFormat`.
@wincent wincent added the feature label Jul 9, 2019
@wincent wincent merged commit 1faf0a1 into master Jul 9, 2019
@wincent wincent deleted the wincent/metal branch July 9, 2019 08:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant