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

Use typescript parser only for typescript files and some small tidying #200

Merged
merged 2 commits into from
Oct 30, 2018

Conversation

BPScott
Copy link
Member

@BPScott BPScott commented Oct 30, 2018

This avoids the react and typescript rulesets potentially overriding
each other regarding setting parsers. Previously if you extended from
'typescript' THEN 'react' you'd get a bunch of errors as the react
ruleset reset the default parser back to babel-eslint.


And some little tidying:

  • Rename rule files to prettier-typescript and react-typescript, as they
    are react/prettier rules that get added when typescript is enabled.
  • Remove lib/config/typescript-prettier and lib/config/typescript-react.
    The configs are still present, but they point to prettier and react
    within the index.js
  • Group configs within index.js to clearly show what is considered a
    core vs augmenting config

This avoids the react and typescript rulesets potentially overriding
each other regarding setting parsers. Previously if you extended from
'typescript' THEN 'react' you'd get a bunch of errors as the react
ruleset reset the default parser back to babel-eslint
* Rename rule files to prettier-typescript and react-typescript, as they
 are react/prettier rules that get added when typescript is enabled.
* Remove lib/config/typescript-prettier and lib/config/typescript-react.
  The configs are still present, but they point to prettier and react
within the index.js
* Group configs within index.js to clearly show what is considered a
core vs augmenting config
@BPScott
Copy link
Member Author

BPScott commented Oct 30, 2018

Previously:

  • Extending react THEN typescript worked, however because typescript extends esnext which extends core, which pulls in rules './rules/shopify' you ended up disabling a selection of react rules, including shopify/jsx-no-complex-expressions, shopify/react-initialize-state and shopify/jest/no-snapshots
  • Extending typescript THEN react results in errors as eslint tried to use the babel-eslint parser on typescript files

After this PR:

  • Extending react THEN typescript worked, however because typescript extends esnext which extends core, which pulls in rules './rules/shopify' you ended up disabling a selection of react rules, including shopify/jsx-no-complex-expressions, shopify/react-initialize-state and shopify/jest/no-snapshots
  • Extending typescript THEN react is now fine, as the typescript rule does not overwrite anything defined in the next config.

I recommend extending typescript THEN react.

Future (major change):

Stop react extending esnext. Treat react as an "augmentation" config like node/jest/lodash etc. That way we stop having extending from two core configs and having them trample over each other.

@BPScott BPScott merged commit 1b4d139 into master Oct 30, 2018
@BPScott BPScott deleted the parser-fight branch October 30, 2018 23:16
BPScott added a commit that referenced this pull request Oct 31, 2018
Imports typescript, then react now that #200 allows for fixing
their import ordering
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants