Skip to content

Compatibility with ESLint 4 #21

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

Closed
BenoitZugmeyer opened this issue Jun 13, 2017 · 22 comments
Closed

Compatibility with ESLint 4 #21

BenoitZugmeyer opened this issue Jun 13, 2017 · 22 comments

Comments

@BenoitZugmeyer
Copy link

Hi, eslint-plugin-html author here. ESLint released its v4, and I released a v3 of my plugin to support it.

I've got some users who are using the vue plugin reporting issues because the html plugin version is fixed to v2. Would you mind changing the html plugin version to v3?

As I understand, you are planing to move away from the html plugin. I don't mind, but I think that in the meantime, users should be allowed to upgrade to ESLint v4.

@michalsnik
Copy link
Member

michalsnik commented Jun 14, 2017

Hi @BenoitZugmeyer. This is a problem indeed, and we should probably fix it, but in the meanwhile we just released eslint-plugin-vue@beta, which basically is 3.0.0 (we merged eslint-plugin-vue-trial with this plugin) and we encourage people to try our new release. It will be the official version as soon as we gather some feedback from users. The sooner the better :) Documentation is still on dev branch but soon it will me on master too.

@yyx990803
Copy link
Member

yyx990803 commented Jun 15, 2017

Ah well it's a bit awkward now because eslint-plugin-html@3 has some breaking changes, so if we bump it it's technically a breaking change for eslint-plugin-vue too, but we just used the 3.0.0 tag
😂

Two options:

  1. deprecate 3.0.0, release 2.x with ESLint 4 compat as 3.0.1;

  2. keep 3.0.0, release 2.x with ESLint 4 compat as 2.1.0.

Option (2) is technically violating semver, but the breaking changes are relatively small and it would probably be less confusion for the users.

@michalsnik
Copy link
Member

Our 3.0.0 resolves this issue and adds new rules so it's the recommended way of getting rid of this problem. But if we want to update previous release I'm opting for 2nd option @yyx990803 proposed to not confuse users too much.

@BenoitZugmeyer is eslint-plugin-html@3.0.0 compatible with both eslint@3.x and eslint@4.x?

@BenoitZugmeyer
Copy link
Author

Yes, compatible with all versions of eslint starting from v2.

@vvo
Copy link

vvo commented Jun 15, 2017

Hi, I just tried to use https://github.com/vuejs/eslint-plugin-vue/tree/dev (@beta) without any luck. Seems like the README is providing an installation procedure that does not work.

There's no recommended.js file inside the node_modules/eslint-plugin-vue

@michalsnik
Copy link
Member

Hey @vvo, you either need to specify 3.0.0 in package.json or do "npm install eslint-plugin-vue@beta". Readme is being updated as we speak. Stay tuned, it should be on master branch soon, probably tomorrow :)

@vvo
Copy link

vvo commented Jun 15, 2017

I did install @beta but then it said it was not able to find eslint-plugin-vue, if you look at the package.json inside the installed npm package, it has a main to lib/index.js but there's no lib/index.js

@michalsnik
Copy link
Member

Try now @vvo, I just merged #23 and released 3.0.1, still as @beta. That's actually weird because I installed it before publishing and it worked nice, but it apparently shouldn't regarding the location of the index.js. Anyway it's now fixed and I hope it works now. Please let me know when you try it.

@vvo
Copy link

vvo commented Jun 16, 2017

Do I need to somehow enforce eslint to also pass on .vue files (like with the html plugin)? If so we should maybe say so in readme?

@vvo
Copy link

vvo commented Jun 16, 2017

I cannot get it work:

> npm run lint

> vue-instantsearch@0.2.1 lint /Users/vvo/Dev/Algolia/vue-instantsearch
> eslint .

Cannot read property 'some' of undefined
TypeError: Cannot read property 'some' of undefined
    at Object.isUnambiguousModule [as isModule] (/Users/vvo/Dev/Algolia/vue-instantsearch/node_modules/eslint-module-utils/unambiguous.js:29:18)
    at Function.ExportMap.parse (/Users/vvo/Dev/Algolia/vue-instantsearch/node_modules/eslint-plugin-import/lib/ExportMap.js:348:20)
    at Function.ExportMap.for (/Users/vvo/Dev/Algolia/vue-instantsearch/node_modules/eslint-plugin-import/lib/ExportMap.js:326:25)
    at Function.ExportMap.get (/Users/vvo/Dev/Algolia/vue-instantsearch/node_modules/eslint-plugin-import/lib/ExportMap.js:284:23)
    at processBodyStatement (/Users/vvo/Dev/Algolia/vue-instantsearch/node_modules/eslint-plugin-import/lib/rules/namespace.js:58:47)
    at Array.forEach (native)
    at Linter.Program (/Users/vvo/Dev/Algolia/vue-instantsearch/node_modules/eslint-plugin-import/lib/rules/namespace.js:87:14)
    at emitOne (events.js:101:20)
    at Linter.emit (events.js:191:7)
    at NodeEventGenerator.applySelector (/Users/vvo/Dev/Algolia/vue-instantsearch/node_modules/eslint/lib/util/node-event-generator.js:265:26)

Eslint project: https://github.com/algolia/eslint-config-algolia/tree/feat/eslint-plugin-vue

You can try in a new repo:

terminal

yarn add \
eslint babel-eslint prettier \
eslint-config-algolia@next eslint-config-prettier \
eslint-plugin-import eslint-plugin-jest eslint-plugin-prettier \
--dev

yarn add eslint-plugin-vue@3.0.1 --dev

.eslintrc.js

module.exports = {
  extends: 'algolia/vue',
};

The try to lint anything, it will fail.

@vvo
Copy link

vvo commented Jun 16, 2017

Also does it lint JavaScript inside <script> code blocks of .vue components?

@mysticatea
Copy link
Member

mysticatea commented Jun 16, 2017

@vvo That error happens in eslint-plugin-import package. That package looks to parse files by itself, it seems to not support parserServices feature of ESLint. You can request the support of parserServices feature to eslint-plugin-import.

Also does it lint JavaScript inside <script> code blocks of .vue components?

Yes. The top-level <script> element are linted.

@michalsnik
Copy link
Member

Regarding .vue files - you need to specify it while linting (eslint . --ext .js,.jsx,.vue) and that's a good point, we should probably mention it in the readme too.

@vvo
Copy link

vvo commented Jun 16, 2017

@mysticatea That's what I thought too but I was wondering why it was failing since the use of eslint-plugin-vue, because inside eslint-plugin-vue we use somme ESLint APIs that are not inside eslint-plugin-import?

eslint-plugin-import is a package that is pretty popular inside the ESLint community, might be worth trying to fix that together?

@mysticatea
Copy link
Member

mysticatea commented Jun 16, 2017

  • The parse() method of parsers which provide parserServices returns an object {ast: object, services: object}. However, eslint-plugin-import assumes that parse() method returns AST.
  • eslint-plugin-vue@3 uses a custom parser which provide parserServices to handle <template> .
  • When eslint-plugin-import uses the custom parser, it returns {ast: object, services: object}, then TypeError happens by the access result.body.some(... since result.body is undefined.

The parserServices feature has been added in ESLint v3.9.0, but I think it has not been documented yet.

@vvo
Copy link

vvo commented Jun 16, 2017

Thanks for the detailed explanation, I am unsure I can fix that myself, maybe @ljharb has more knowledge on this parse and parserServices API mismatch between those two eslint plugins.

In the meantime, any way I can still use eslint-plugin-vue, maybe by disabling the import plugin on vue files or smg?

@ljharb
Copy link

ljharb commented Jun 16, 2017

If it's not a documented API, and nobody's requested it before on eslint-plugin-import, then naturally we'd have no idea about it.

Please feel free to file an issue and/or a PR on eslint-plugin-import.

Separately, the HTML plugin should not violate semver; it should backport v4 compat to the 2.x line without additional changes. Violating semver will never reduce user confusion.

mysticatea added a commit that referenced this issue Jun 18, 2017
- Make correct ASTs about self-closing elements (fixes #29)
- Improve the integration with eslint-plugin-import (refs #21)
mysticatea added a commit that referenced this issue Jun 18, 2017
* Fix: upgrade vue-eslint-parser
  - Make correct ASTs about self-closing elements (fixes #29)
  - Improve the integration with eslint-plugin-import (refs #21)
* add comments to integration test
@mysticatea
Copy link
Member

mysticatea commented Jun 18, 2017

I made a workaround for eslint-plugin-import in vue-eslint-parser. I expect the integration problem is solved in the next patch release.

@michalsnik
Copy link
Member

Problem with eslint-plugin-import plugin should be fixed in: v3.1.1 which I just released. I also released v2.1.0 which now supports eslint-plugin-html and Eslint 4.

@michalsnik
Copy link
Member

Feel free to reopen this issue if you have more questions.

@vvo
Copy link

vvo commented Jun 19, 2017

Hi I am having a different issue now that this one was fixed:

image

I guess that one just means that prettier is unable to go through those template tags. We are using prettier on the whole codebase, I don't know the state of this for now.

prettier/prettier#2097
prettier/prettier#1674

cc @vjeux I guess prettier is unable to format <template> nowadays while our eslint configuration uses eslint-plugin-prettier and try to lint .vue and .js files? Thanks

@vjeux
Copy link

vjeux commented Jun 19, 2017

@vvo: we're working on printing vue files (both the html part of it and the inline js) but it's not released (only in master) and very early on. You should find a way to only run prettier on js files for now or help out with the printing of vue files.

Hopefully in the next few weeks proper vue support should be good to go. I'm pretty excited about it :)

doug-wade pushed a commit to doug-wade/eslint-plugin-vue that referenced this issue Apr 17, 2022
Fix bug where lists weren't getting converted to comma separated strings before calling API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants