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

fix: don't break when using @typescript-eslint/parser #153

Merged
merged 3 commits into from
Feb 24, 2020

Conversation

wincent
Copy link
Contributor

@wincent wincent commented Feb 24, 2020

The sort-import-destructures rule was working fine in liferay-portal and in our local test suite, but I discovered while updating the version of eslint-config-liferay used in Clay to v19.0.1 that the rule's autofixer was mangling the source because the start and end properties are undefined when using the @typescript-eslint/parser parser.

range[0] and range[1], however, are equivalent, and are set by both @typescript-eslint/parser and the standard (espree) parser.

Test plan: In addition to manual fixing, adjusted the tests to always run against both espree and @typescript-eslint/parser. Sample failure output looks like this (note the "parser:" annotation):

FAIL  plugins/eslint-plugin-liferay/tests/lib/rules/sort-import-destructures.js
  ● sort-import-destructures (parser: typescript) › invalid › example 1: import {z, g} from 'a';

    assert(received)

    Expected value to be equal to:
      true
    Received:
      false

The sort-import-destructures rule was working fine in liferay-portal and
in our local test suite, but I discovered while updating the version of
eslint-config-liferay used in Clay to v19.0.1 that the rule's autofixer
was mangling the source because the `start` and `end` properties are
undefined when using the `@typescript-eslint/parser` parser.

`range[0]` and `range[1]`, however, are equivalent, and are set by both
`@typescript-eslint/parser` and the standard parser.
This will ensure that bugs like the one in the parent commit don't creep
back in. Sample failure output looks like this (note the "parser:"
annotation):

  FAIL  plugins/eslint-plugin-liferay/tests/lib/rules/sort-import-destructures.js
    ● sort-import-destructures (parser: typescript) › invalid › example 1: import {z, g} from 'a';

      assert(received)

      Expected value to be equal to:
        true
      Received:
        false
With `npx yarn-deduplicate yarn.lock`.
@wincent wincent added the bug label Feb 24, 2020
constructor(options) {
super(options);

this._liferay = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using this "namespace" here to avoid possibly clobbering a current or future property with the name _parsers.

@wincent
Copy link
Contributor Author

wincent commented Feb 24, 2020

Also tested this in Clay, producing this diff, so I think it's good to merge.

@wincent wincent merged commit a5767cc into master Feb 24, 2020
@wincent wincent deleted the wincent/fix-sort-import-destructure branch February 24, 2020 13:55
bryceosterhaus pushed a commit to bryceosterhaus/clay that referenced this pull request May 19, 2020
Updates to eslint-config-liferay v19.0.2 which brings in some new rules:

- curly (enforces use of curlies around conditional blocks).
- liferay/array-is-array (enforces use of `Array.isArray` instead of
  `instanceof Array`).
- liferay/import-extensions (prohibits superflous extensions in imports).
- liferay/sort-import-destructures (yet another in the family of
  "sorting" rules).
- padding-line-between-statements (specifically, requiring a blank line
  before `return` statements, at Brian's request).
- react/forbid-foreign-prop-types (so that we can be sure that stripping
  propTypes in production builds is safe; obviously won't be relevant in
  this repo).

and produces these new lint errors (about 67 of them):

https://gist.github.com/wincent/84bbd941eec684218acb21ad34fdb156

All of those have autofixes, so I applied them.

I also removed some cruft from the config file (values that are not
needed because they are the defaults).

Note that in testing this, I discovered that the
new "liferay/sort-import-destructures" rule and the
`@typescript-eslint/parser` parser were not playing nicely together. I
first tried updating some of the related NPM packages (ESLint, the
parser etc) but that didn't work (although it did require me to
update one config value), so I then dug deeper and tweaked the rule
implementation to work with both the `@typescript-eslint/parser` and
"espree" (ie. default) parsers:

liferay/eslint-config-liferay#153
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