search

Unibeautify / beautifier-eslint

ESLint beautifier for Unibeautify
MIT License
3 stars 1 forks source link

Error: Failed to load plugin: xxx #87

Open muuvmuuv opened 6 years ago

muuvmuuv commented 6 years ago

I'm using VSCode with the current version of Uniebeautify and have ESLint enabled with prefer_beautifier_config option. Now I'm working on a react project with gatsby (I'm using this template: https://github.com/haysclark/gatsby-starter-casper) and got the following error (copied form the dev console):

[Extension Host] FormattingOptions {tabSize: 2, insertSpaces: true}
console.ts:136 [Extension Host] beautifyData {fileExtension: ".js", filePath: "/Users/marvinheilemann/Documents/Cookie Soft/Website/new/data/SiteConfig.js", languageName: "JavaScript", options: {…}, projectPath: "/Users/marvinheilemann/Documents/Cookie Soft/Website/new", …}
console.ts:136 [Extension Host] Error: Failed to load plugin react: Cannot find module 'eslint-plugin-react'
    at Function.Module._resolveFilename (module.js:543:15)
    at Function.resolve (internal/module.js:18:19)
    at Plugins.load (/Users/marvinheilemann/.vscode/extensions/glavin001.unibeautify-vscode-0.6.1/node_modules/eslint/lib/config/plugins.js:106:29)
    at Array.forEach (<anonymous>)
    at Plugins.loadAll (/Users/marvinheilemann/.vscode/extensions/glavin001.unibeautify-vscode-0.6.1/node_modules/eslint/lib/config/plugins.js:166:21)
    at loadFromDisk (/Users/marvinheilemann/.vscode/extensions/glavin001.unibeautify-vscode-0.6.1/node_modules/eslint/lib/config/config-file.js:501:35)
    at Object.load (/Users/marvinheilemann/.vscode/extensions/glavin001.unibeautify-vscode-0.6.1/node_modules/eslint/lib/config/config-file.js:559:20)
    at Config.getLocalConfigHierarchy (/Users/marvinheilemann/.vscode/extensions/glavin001.unibeautify-vscode-0.6.1/node_modules/eslint/lib/config.js:227:44)
    at Config.getConfigHierarchy (/Users/marvinheilemann/.vscode/extensions/glavin001.unibeautify-vscode-0.6.1/node_modules/eslint/lib/config.js:179:43)
    at Config.getConfigVector (/Users/marvinheilemann/.vscode/extensions/glavin001.unibeautify-vscode-0.6.1/node_modules/eslint/lib/config.js:286:21)
    at Config.getConfig (/Users/marvinheilemann/.vscode/extensions/glavin001.unibeautify-vscode-0.6.1/node_modules/eslint/lib/config.js:329:29)
    at CLIEngine.getConfigForFile (/Users/marvinheilemann/.vscode/extensions/glavin001.unibeautify-vscode-0.6.1/node_modules/eslint/lib/cli-engine.js:653:29)
    at Object.resolveConfig (/Users/marvinheilemann/.vscode/extensions/glavin001.unibeautify-vscode-0.6.1/node_modules/@unibeautify/beautifier-eslint/src/index.ts:37:44)
    at dependencyManager.load.then (/Users/marvinheilemann/.vscode/extensions/glavin001.unibeautify-vscode-0.6.1/node_modules/unibeautify/src/beautifier.ts:400:35)
    at <anonymous>

I think it is because the beautifier plugin is using its own or the global ESLint installation? I have one installed in my project with the plugin it needs. Maybe this can be fixed when Unibeautify is checking whenever a plugins CI is already installed in 1. project 2. global and then fall back to the local installation.

Glavin001 commented 6 years ago

Pull Requests welcome!

I think adding official support to beautifier-eslint for React makes sense.

  1. Add https://www.npmjs.com/package/eslint-plugin-react as a package.json dependencies
  2. Add eslint-plugin-react as a Dependency to https://github.com/Unibeautify/beautifier-eslint/blob/master/src/index.ts#L21-L25
  3. Add eslint-plugin-react to CLIEngine ( https://github.com/Unibeautify/beautifier-eslint/blob/master/src/index.ts#L53 ) so ESLint can use the plugin. See https://eslint.org/docs/developer-guide/nodejs-api#cliengineaddplugin
  4. Add support for language JSX: https://github.com/Unibeautify/beautifier-eslint/blob/master/src/index.ts#L28

Let me know if you have any questions!

Applicable resources:

cc @szeck87

muuvmuuv commented 6 years ago

@Glavin001 Okay I'll try to do this. btw. I think it would be useful to give an extra parameter to the dependency object to specify which is the core package and which are plugins or something else.

muuvmuuv commented 6 years ago

And I thought about another object in the Beautifier interface like "addPlugin" which returns the program containing all plugins. Just to separate those things.

Glavin001 commented 6 years ago

Plugins (like ESLint Plugins) are an implementation detail of the specific beautifier (i.e. ESLint), not of Unibeautify. A Unibeautify Beautifier should be complete with all applicable plugins to support the languages (e.g. JavaScript, JSX) and options.

stevenzeck commented 6 years ago

@Glavin001 should eslint-plugin-react be a dependency or a peer dependency? I think it should be optional since not everyone will use beautifier-eslint for JSX.

Glavin001 commented 6 years ago

@szeck87 I think peerDependency makes sense: https://github.com/npm/npm/issues/3066#issuecomment-154161544

muuvmuuv commented 6 years ago

@Glavin001 I thought about something else to make it more pluggable.

beautifier-eslint is using ESLint executor from its node_modules, so it is not possible to load plugins through a projects .eslintrc.json, so beautifier-eslint would need to install them in his node_modules. What if we would use the projects eslint when some plugins are used. So we are noticing the user "We have recognised that you are using eslint plugins in your config. To use them you need to install eslint, so beautifier-eslint can use this executable". This vscode plugin does a similiar thing: https://github.com/Microsoft/vscode-eslint/blob/master/client/src/utils.ts

I will try some things locally and hope to get this working.

Glavin001 commented 6 years ago

The issue is the Unibeautify Beautifier needs to be aware of all of the languages and options it supports. So by adding eslint-plugin-react there are more languages, such as JSX, and more options, any React specific rules. These need to be known ahead of time.

What if we would use the projects eslint when some plugins are used.

I want to support loading eslint and other Node Dependencies from:

  1. Project node_modules/
  2. Globally installed modules
  3. VSCode editor extension preinstalled node_modules

This should be handled by https://github.com/Unibeautify/unibeautify/blob/master/src/DependencyManager/NodeDependency.ts and potentially https://github.com/Unibeautify/vscode I think Unibeautify's NodeDependency may still need some work to support all of the above.

I will try some things locally and hope to get this working.

I do not recommend investigating anything experimental like this until we've discussed it fully. I think you're making great progress on https://github.com/Unibeautify/beautifier-eslint/pull/88 and both @szeck87 and I have made comments. Let's get https://github.com/Unibeautify/beautifier-eslint/pull/88 merged and then we can investigate how to make it even better.

Glavin001 commented 6 years ago

🎉 https://github.com/Unibeautify/beautifier-eslint/pull/88 has been merged. 🎆

muuvmuuv commented 6 years ago

@Glavin001: I thought about somethings yesterday evening. We could look for the package.json that is next to the file that will be formatted or the projectdir. Then we search for matches with eslint-plugin-*. If there're any plugins, include them with NodeDependencies and tag an eslint Dependencies as its parent. After that loop them and add them to the CLI.

Glavin001 commented 6 years ago

This would add plugins dynamically to ESLint's Engine, however, the bigger problem is the Beautifier's supported options and languages. Your PR #88 worked well with having eslint-plugin-react as an optional: true dependency and checking if it was installed -- similar to checking package.json as you described above, except it checks for node_modules/.

Now that I think of it, I do have a problem with #88 in that the eslint-plugin-react was optional except JSX language will always be shown as supported by the beautifier, even if eslint-plugin-react was not loaded. We may have to think about this a little bit more. cc @szeck87 . If the language is JSX and eslint-plugin-react is not installed, should this throw an error? 🤔

muuvmuuv commented 6 years ago

Maybe we will let this open and rethink about NodeDependencies first. There should be a change to make it more dynamic when working with plugins.

Glavin001 commented 6 years ago

Agreed. I'll leave this issue open :).

muuvmuuv commented 6 years ago

@Glavin001 You need to update the package version to let npm update the package in e.g. the website. Currently I get Error: Cannot find module 'eslint-plugin-react' because v0.5.0 has it not set a peerdependeny :)

Glavin001 commented 6 years ago

You're right. We need to publish a new release with a version bumped to 0.6.0. Current version 0.5.0 was published maybe 5 months ago 🔥 :

Coming soon 😄 . Thanks, @muuvmuuv !

Glavin001 commented 6 years ago

Published to v0.6.0! 🎉

muuvmuuv commented 6 years ago

@Glavin001 may give it a tag 0.6.0 to let GitHub make a downloadable package? I think we should always add minor version updates to a version tag.

Glavin001 commented 6 years ago

Good thinking. I was in a rush earlier. Created a new GitHub Release: https://github.com/Unibeautify/beautifier-eslint/releases/tag/v0.6.0