Skip to content
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

Override parser to babylon - Fix compatibility with eslint-plugin-html and [email protected] #98

Closed
nappy opened this issue Jul 2, 2018 · 8 comments · Fixed by #111

Comments

@nappy
Copy link

nappy commented Jul 2, 2018

Upgrading prettier from ~1.12.1 to ^1.13.6 causes eslint-plugin-html to break.

The use case is, to lint and format javascript inside html script tags.

I originally posted the issue at prettier/prettier#4759, but it became clear that it should be fixed here.

Prettier 1.13 changes how the parser is inferred, previously it would default to babylon, but now this must be explicitly defined in the .prettierrc config.
Since eslint only lints javascript it would make sense to restore the previous behavior by overriding the parser to babylon when calling the prettier API (if possible), and therefore restore compatibility with previous working configurations.

Please read the referenced issue, if its not clear from this description.

Environments:

  • Prettier Version: 1.13.6
  • Running Prettier via: eslint-plugin-prettier
  • Runtime: Node v10.5.0 Yarn 1.7.0 Npm 6.1.0
  • Operating System: Windows
    eslint@^5.0.1
    eslint-plugin-html@^4.0.5
    eslint-plugin-prettier@^2.6.1
    prettier@^1.13.6

Steps to reproduce:
Add plugins html and prettier to .eslintrc
Add an html file to your project
Run eslint **/*.{js,html}

Expected behavior: (with prettier 1.12.1)
yarn run v1.7.0
$ eslint **/*.{js,html}
Done in 5.89s.

Actual behavior: (with prettier 1.13.6)
yarn run v1.7.0
$ eslint **/*.{js,html}
No parser could be inferred for file: C:\Code\eslint-html\assets\index.html
Error: No parser could be inferred for file: C:\Code\eslint-html\assets\index.html
at normalize (C:\Code\eslint-html\node_modules\prettier\index.js:7058:15)
at formatWithCursor (C:\Code\eslint-html\node_modules\prettier\index.js:10378:12)
at C:\Code\eslint-html\node_modules\prettier\index.js:31217:15
at Object.format (C:\Code\eslint-html\node_modules\prettier\index.js:31236:12)
at Program (C:\Code\eslint-html\node_modules\eslint-plugin-prettier\eslint-plugin-prettier.js:412:45)
at listeners.(anonymous function).forEach.listener (C:\Code\eslint-html\node_modules\eslint\lib\util\safe-emitter.js:45:58)
at Array.forEach ()
at Object.emit (C:\Code\eslint-html\node_modules\eslint\lib\util\safe-emitter.js:45:38)
at NodeEventGenerator.applySelector (C:\Code\eslint-html\node_modules\eslint\lib\util\node-event-generator.js:251:26)
at NodeEventGenerator.applySelectors (C:\Code\eslint-html\node_modules\eslint\lib\util\node-event-generator.js:280:22)
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@not-an-aardvark
Copy link
Member

Thanks for the report. I think this is roughly the same issue as #81.

Unfortunately, I'm not sure always using babylon would be a good idea, because some users are depending on having a different parser get used when they lint ,vue files. That said, I'm open to any other suggestions you have for how to fix this.

@nappy
Copy link
Author

nappy commented Jul 2, 2018

Maybe you can only set the parser explicitly, if there is no other parser defined in .prettierrc? Personally I am not involved in any of those three projects, so its not really up to me to come up with suggestions here.

It also might be the case that eslint-plugin-html as plugin is not really supported by eslint
in the first place, because its API (might?) only supports full files? Again I am no expert here, Maybe @BenoitZugmeyer can help out?

As a user I can only say that its not good, that things break with no major version bump. And even if its because slightly unsupported API usage, It would be good to come up with a fix. On the other hand I am not sure how many users are actually affected by this. With polymer's migration away from html imports, maybe its not that important for the future anyways. So maybe if there is no clean fix, its better to just change the .prettierrc on the users end.

@BPScott
Copy link
Member

BPScott commented Sep 26, 2018

As already mentioned this is is basically the same as #81. The fix is essentially as that issue, see #81 (comment) for deeper details on how eslint-plugin-prettier interacts other plugins that provide processors.

The fix for this is to stop running the prettier/prettier rule over the files that eslint-plugin-html provides a processor for as eslint-plugin-html does not support autofixing prettier issues

Add an overrides section to the bottom of your eslint config, after your rules: {} like so:

{
  "rules": {},
  "overrides": [
    {
      "files": ["*.html"],
      "rules": {
        "prettier/prettier": "off"
      }
    }
  ]
}

@BenoitZugmeyer
Copy link

The next version of eslint-plugin-html will introduce a new setting html/fake-file-extension, so you'll be able to fake the file extenson to tell prettier to use the right parser.

See my previous comment on the subject: prettier/prettier#4759 (comment)

as eslint-plugin-html does not support autofixing prettier issues

What makes you say this? The plugin support autofixing any ESLint rule, so it should support autofixing prettier rules too.

@BPScott
Copy link
Member

BPScott commented Sep 26, 2018

What makes you say this? The plugin support autofixing any ESLint rule, so it should support autofixing prettier rules too.

https://eslint.org/docs/developer-guide/working-with-plugins says supportsAutofix: true must be added to a processor's declaration in order for rules to be autofixable. I did not see that declaration in eslint-plugin-html: https://github.com/BenoitZugmeyer/eslint-plugin-html/search?q=supportsAutofix&unscoped_q=supportsAutofix

@BenoitZugmeyer
Copy link

Ha, but my plugin is 🌟 magic 🌟 Long story short: as soon as the plugin is loaded, I am patching some ESLint libraries, and don't follow standard plugin conventions at all. This allows me to:

  • Have access to the current configuration used (preprocessors don't have access to this)
  • Allow the user to configure which extensions needs to be considered as HTML or XHTML (HTML can be found in a huge variety of file extensions)
  • Run the ESLint verification multiple times by file (= once on each script tag)
  • Support ESLint autofix long before the supportsAutofix flag was introduced

This is indeed harder to maintain but I have not too many complaints.

@BPScott
Copy link
Member

BPScott commented Sep 26, 2018

Cool, I didn't realize you were going off-piste. Thanks for the context.

I've just opened #111 which should make prettier use the babylon parser for unknown file types (and preemptively for files it would try to parse with its html parser when that gets released in v1.15).

Hopefully if that gets merged then people using both eslint-plugin-prettier and eslint-plugin-html won't need to specify your fake-file-extension.

not-an-aardvark pushed a commit that referenced this issue Sep 26, 2018
* Ignore files in .prettierignore

Fixes #88

* Force the babylon parser when parsing not-js files

Forcing the parser stops errors that are caused by trying to run a JS
fragment through the graphql / markdown parsers.

The 'html' parser has not yet been released yet, but will be in Prettier
1.15.

This uses a block list over an allow list because I expect the list of
"file types with a prettier parser that could contain javascript fragments"
will grow at a slower pace than "file types with a prettier parser that
are variations of the javascript language".

Fixes #98, Fixes #81
@BenoitZugmeyer
Copy link

This is a nice solution. I guess I won't release the fake-file-extension, then. Cheers!

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.

4 participants