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

ESLint setup in VS Code #20996

Closed
jeherve opened this issue Sep 8, 2021 · 18 comments · Fixed by #23568
Closed

ESLint setup in VS Code #20996

jeherve opened this issue Sep 8, 2021 · 18 comments · Fixed by #23568
Assignees
Labels
[Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended [Type] Infrastructure

Comments

@jeherve
Copy link
Member

jeherve commented Sep 8, 2021

There are 2 main ways to lint JS files in our monorepo:

  1. You can rely on pnpm lint-file filename command to lint a file from your terminal.
  2. You can get ESLint suggestions right in your IDE, as you're working on the code.

For some reason, the latter doesn't seem to work in VS Code anymore.

I wonder if it could be related to our switch to PNPM and its workspaces, or to the changes we made in #20085.

Steps to reproduce

  1. Install VS Code
  2. Install the ESLint extension ( https://github.com/microsoft/vscode-eslint )
  3. In a new VS Code window, File > Open... to open the folder where your Jetpack monorepo checkout lives.
  4. Go to File > Add Folder to workspace to create a new workspace for Jetpack.
  5. Once you've done so, hit Cmd + Shift + P to open the quick dialog, and search for "Preferences Workspace JSON", and open that.
  6. There you'll be able to add new ESLint settings for your workspace, inside the settings property. Here is what I have set on my end for example:
"eslint.workingDirectories": [
  "/Users/jeherve/code/jetpack",
  "/Users/jeherve/code/jetpack/tools/js-tools/",
  "/Users/jeherve/code/jetpack/projects/plugins/boost/",
  "/Users/jeherve/code/jetpack/projects/plugins/jetpack/",
  "/Users/jeherve/code/jetpack/projects/plugins/jetpack/extensions/",
  "/Users/jeherve/code/jetpack/projects/plugins/jetpack/modules/search/",
  "/Users/jeherve/code/jetpack/projects/plugins/backup/",
],
"eslint.alwaysShowStatus": true,
"eslint.packageManager": "pnpm",
"eslint.runtime": "node",
"eslint.lintTask.enable": true,
"eslint.format.enable": true,
"eslint.probe": [
    "javascript",
    "javascriptreact",
    "typescript",
    "typescriptreact",
    "html",
    "markdown"
],
  1. Once you've set that up, hit Cmd + Shift + P again and search for "eslint server"; you'll have the option to restart the server. Do that.
  2. At the bottom of the editor, you'll see an output tab; in there, select the ESLint dropdown to get the ESLint output:

image

Here is the output on my end today:

[Info  - 5:29:05 PM] ESLint server is starting
[Info  - 5:29:05 PM] ESLint server running in node v16.7.0
[Info  - 5:29:05 PM] ESLint server is running.
[Info  - 5:29:05 PM] ESLint library loaded from: /Users/jeherve/code/jetpack/node_modules/.pnpm/eslint@7.31.0/node_modules/eslint/lib/api.js
Uncaught exception received.
Error: Failed to load config "wpcalypso/react" to extend from.
Referenced from: /Users/jeherve/code/jetpack/.eslintrc.react.js
    at configInvalidError (/Users/jeherve/code/jetpack/node_modules/.pnpm/@eslint+eslintrc@0.4.3/node_modules/@eslint/eslintrc/lib/config-array-factory.js:290:9)
    at ConfigArrayFactory._loadExtendedShareableConfig (/Users/jeherve/code/jetpack/node_modules/.pnpm/@eslint+eslintrc@0.4.3/node_modules/@eslint/eslintrc/lib/config-array-factory.js:883:23)
    at ConfigArrayFactory._loadExtends (/Users/jeherve/code/jetpack/node_modules/.pnpm/@eslint+eslintrc@0.4.3/node_modules/@eslint/eslintrc/lib/config-array-factory.js:781:25)
    at ConfigArrayFactory._normalizeObjectConfigDataBody (/Users/jeherve/code/jetpack/node_modules/.pnpm/@eslint+eslintrc@0.4.3/node_modules/@eslint/eslintrc/lib/config-array-factory.js:720:25)
    at _normalizeObjectConfigDataBody.next (<anonymous>)
    at ConfigArrayFactory._normalizeObjectConfigData (/Users/jeherve/code/jetpack/node_modules/.pnpm/@eslint+eslintrc@0.4.3/node_modules/@eslint/eslintrc/lib/config-array-factory.js:665:20)
    at _normalizeObjectConfigData.next (<anonymous>)
    at ConfigArrayFactory._normalizeObjectConfigDataBody (/Users/jeherve/code/jetpack/node_modules/.pnpm/@eslint+eslintrc@0.4.3/node_modules/@eslint/eslintrc/lib/config-array-factory.js:720:25)
    at _normalizeObjectConfigDataBody.next (<anonymous>)
    at ConfigArrayFactory._normalizeObjectConfigData (/Users/jeherve/code/jetpack/node_modules/.pnpm/@eslint+eslintrc@0.4.3/node_modules/@eslint/eslintrc/lib/config-array-factory.js:665:20)
Uncaught exception received.

Previous conversations on the topic:

  • p1629911000326300-slack-CBG1CP4EN (cc @retrofox )
  • p1625223097173000-slack-CBG1CP4EN (cc @brbrr )
  • p1627994576016900-slack-C019N9JKKM5 (cc @davidlonjon )
@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Pri] Normal [Type] Infrastructure labels Sep 8, 2021
@jeherve
Copy link
Member Author

jeherve commented Sep 8, 2021

@retrofox @brbrr @davidlonjon Is this still an issue for you?

@brbrr
Copy link
Contributor

brbrr commented Sep 8, 2021

Yeah, I'm facing the same issue when trying to save changes in Jetpack's JS code

@brbrr
Copy link
Contributor

brbrr commented Sep 8, 2021

and I think @sdixon194 was experiencing something similar recently

@davidlonjon
Copy link
Contributor

davidlonjon commented Sep 9, 2021

Hi @jeherve,

I actually always had a very similar issue (since I setup Jetpack Monorepo for the first time) but with PHPStorm as shown in the following screenshots:

When on the .eslintrc.js file inside the Jetpack plugin directory:
image

When on a JS file inside the Jetpack Boost plugin directory:
image

Today I am still experiencing it in PHPStorm. I did not have the opportunity to try it on VSCode but I guess the root of the issue for both IDE is related.

@davidlonjon
Copy link
Contributor

@jeherve,

After trying out a few things, it seems that adding a .npmrc file containing the following on the root of the projects/plugins/boost folder solves the eslint issue in PHPStorm for Boost TypeScript files (though it does not lint svelte files, but this is another issue I think).

public-hoist-pattern[]=*eslint*
public-hoist-pattern[]=*wpcalypso*
public-hoist-pattern[]=*wordpress*

I tried to add them on the .npmrc file on the root of the Jetpack Monorepo directory as

public-hoist-pattern=['*types*', '@prettier/plugin-*', '*prettier-plugin-*', '*eslint*', '*wpcalypso*', '*wordpress*']

without having the projects/plugins/boost/.npmrc file but it did not fix the issue.

So while having a projects/plugins/boost/.npmrc file as described above, I then did

  • Closed PHPStorm
  • From the Jetpack Monorepo root folder:
$ rm -rf node_modules && rm -rf projects/plugins/boost/node_modules
$ pnpm install && jetpack install plugins/boost
  • Reopened PHPStorm and the issue with eslint on the TypeScript files went away.

After doing the commands above the node_modules at the root of the Jetpack Monorepo looked like this:

jetpack_node_modules_after

However before the fix, the folder looked like:

jetpack_node_modules_before

So it seems the public-hoist-pattern additions in the newly created projects/plugins/boost/.npmrc file allowed to have the symlinks of the eslint related packages and few other ones to be created in the root of the node_modules folder and PHPStorm and Eslint can find the packages. It might have the same effect on VSCode.

Note the content of the projects/plugins/boost/node_modules content as shown below did not change before and after the fix I described

jetpack_boost_node_modules_after

I don't know much about PNPM and how it works as well as Eslint. I am actually if this fix is just a dirty hack and might not be too relevant to try to find a final solution but hopefully, this can help.

@jeherve
Copy link
Member Author

jeherve commented Sep 9, 2021

Interesting, nice catch! You would expect the link to jetpack-js-tools to be enough, but I suppose that's not working quite right!

though it does not lint svelte files, but this is another issue I think

Yes, we'd have to add svelte to the configuration, just like I did for Typescript in #20537.

@renatoagds
Copy link
Contributor

Just pushing an update in top of this. I fixed this problem exporting the NODE_PATH auto-generated from PNPM.

It's located at: tools/js-tools/node_modules/.bin/eslint, my NODE_PATH was this one:

export NODE_PATH="/Users/renato.santos/Documents/Code/jetpack/node_modules/.pnpm/eslint@7.32.0/node_modules/eslint/bin/node_modules:/Users/renato.santos/Documents/Code/jetpack/node_modules/.pnpm/eslint@7.32.0/node_modules/eslint/node_modules:/Users/renato.santos/Documents/Code/jetpack/node_modules/.pnpm/eslint@7.32.0/node_modules:/Users/renato.santos/Documents/Code/jetpack/node_modules/.pnpm/node_modules:/Users/renato.santos/Documents/Code/jetpack/node_modules:/Users/renato.santos/Documents/Code/node_modules:/Users/renato.santos/Documents/node_modules:/Users/renato.santos/node_modules:/Users/node_modules:/node_modules:/Users/renato.santos/Documents/Code/jetpack/tools/js-tools/node_modules:/Users/renato.santos/Documents/Code/jetpack/tools/node_modules"

I don't think is the best solution, since this is auto-generated and could change easily, but it makes the eslint work again.

Will continue to search more solutions for that, since it's related with PNPM way of hoist deps (thanks @anomiex for point that).

@roundhill
Copy link
Contributor

Just chiming in that I have this problem as well :D

@manzoorwanijk
Copy link
Member

manzoorwanijk commented Feb 7, 2022

Facing a similar issue, VS Code output says

["ERROR" - 3:42:02 PM] Error formatting document.
["ERROR" - 3:42:02 PM] Cannot find module 'prettier-plugin-svelte'

@jeherve
Copy link
Member Author

jeherve commented Feb 8, 2022

Here is the full error with Prettier for me:

["ERROR" - 1:21:31 PM] Error formatting document.
["ERROR" - 1:21:31 PM] Cannot find module 'prettier-plugin-svelte'
Require stack:
- /Users/jeherve/code/jetpack/node_modules/.pnpm/wp-prettier@2.0.5/node_modules/wp-prettier/index.js
- /Users/jeherve/.vscode/extensions/esbenp.prettier-vscode-9.2.0/dist/extension.js
- /Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/loader.js
- /Applications/Visual Studio Code.app/Contents/Resources/app/out/bootstrap-amd.js
- /Applications/Visual Studio Code.app/Contents/Resources/app/out/bootstrap-fork.js
Error: Cannot find module 'prettier-plugin-svelte'
Require stack:
- /Users/jeherve/code/jetpack/node_modules/.pnpm/wp-prettier@2.0.5/node_modules/wp-prettier/index.js
- /Users/jeherve/.vscode/extensions/esbenp.prettier-vscode-9.2.0/dist/extension.js
- /Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/loader.js
- /Applications/Visual Studio Code.app/Contents/Resources/app/out/bootstrap-amd.js
- /Applications/Visual Studio Code.app/Contents/Resources/app/out/bootstrap-fork.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:934:15)
    at h.resolve (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/loader.js:4:761)
    at /Users/jeherve/code/jetpack/node_modules/.pnpm/wp-prettier@2.0.5/node_modules/wp-prettier/index.js:51643:21
    at Array.map (<anonymous>)
    at Object.load (/Users/jeherve/code/jetpack/node_modules/.pnpm/wp-prettier@2.0.5/node_modules/wp-prettier/index.js:51635:61)
    at Object.load [as loadPlugins] (/Users/jeherve/code/jetpack/node_modules/.pnpm/wp-prettier@2.0.5/node_modules/wp-prettier/index.js:16867:26)
    at /Users/jeherve/code/jetpack/node_modules/.pnpm/wp-prettier@2.0.5/node_modules/wp-prettier/index.js:51709:28
    at Object.Success [as format] (/Users/jeherve/code/jetpack/node_modules/.pnpm/wp-prettier@2.0.5/node_modules/wp-prettier/index.js:51731:12)
    at t.default.format (/Users/jeherve/.vscode/extensions/esbenp.prettier-vscode-9.2.0/src/PrettierEditService.ts:435:45)
    at t.PrettierEditProvider.provideEdits (/Users/jeherve/.vscode/extensions/esbenp.prettier-vscode-9.2.0/src/PrettierEditService.ts:322:22)
    at A.provideDocumentFormattingEdits (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/services/extensions/node/extensionHostProcess.js:95:95214)
["INFO" - 1:21:31 PM] Formatting completed in 0.065ms.

@yansern
Copy link
Contributor

yansern commented Feb 14, 2022

Having the same problem here with VSCode. Using pnpm lint-file --fix file-here for now.

@manzoorwanijk
Copy link
Member

Why is plugins: [ 'prettier-plugin-svelte' ] added to .prettierrc.js in the root directory?

I think it belongs to .prettierrc.js of the package that uses Svelte.

@jeherve
Copy link
Member Author

jeherve commented Mar 22, 2022

It was added in #21568. We currently do not have a cascading Prettier configuration with multiple config files for each plugin / project in the monorepo. It may be worth giving it a try indeed, that seems logical, although I do not know if Prettier currently supports that kind of cascading setup?

@manzoorwanijk
Copy link
Member

I do not know if Prettier currently supports that kind of cascading setup?

We do this this in root .prettierrc.js

{
	useTabs: true,
	tabWidth: 2,
	printWidth: 100,
	singleQuote: true,
	trailingComma: 'es5',
	bracketSpacing: true,
	parenSpacing: true,
	jsxBracketSameLine: false,
	semi: true,
	arrowParens: 'avoid',
}

Then in package specific .prettierrc.js, for example plugins/boost, we can use this

{
	...require('../../../.prettierrc.js'),
	plugins: [ 'prettier-plugin-svelte' ],
	svelteStrictMode: false,
	svelteBracketNewLine: true,
	svelteIndentScriptAndStyle: true,
	svelteSortOrder: 'options-scripts-styles-markup',
}

@manzoorwanijk
Copy link
Member

Also, with regards to ESLint, extending a base config doesn't mean that the dependencies (eslint plugins) will be used from the base config location, those would rather be looked at the location (node_modules) in the same directory as the derived config file. Please see Notes here https://eslint.org/docs/user-guide/configuring/plugins#configuring-plugins

So, it means if we extend any ESLint config, we will have to add all the underlying dependencies as well.

@anomiex
Copy link
Contributor

anomiex commented Mar 22, 2022

So, it means if we extend any ESLint config, we will have to add all the underlying dependencies as well.

While the documentation does say that, in practice we already have various plugins that are only installed within tools/js-tools/ that are being loaded without issue. I haven't dug too deeply into why, although I suspect it's either NODE_PATH or pnpm's hoisting into node_modules/.pnpm/ which is the root of all the node_modules symlinks' targets.

@manzoorwanijk
Copy link
Member

manzoorwanijk commented Mar 23, 2022

in practice we already have various plugins that are only installed within tools/js-tools/ that are being loaded without issue

CLI and IDE work differently. When using ESLint CLI, plugins are loaded relative to CWD but when using npm packages (used by IDEs), plugins are loaded relative to config files.

--resolve-plugins-relative-to path:String A folder where plugins should be resolved from, CWD by default

https://eslint.org/docs/user-guide/command-line-interface#options

@manzoorwanijk
Copy link
Member

The issue is much deeper than it looks. There is a feature request which is still open eslint/eslint#3458. The current state of ESLint doesn't work for monorepos as we expect it to...

Good news is that there is a workaround exactly for issue we are facing - @rushstack/eslint-patch. I tested it and it works with minimal changes.

Will try to create a PR today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended [Type] Infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants