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

[Discussion] IntelliJ and Yarn 2 integration #499

Closed
segrey opened this issue Sep 30, 2019 · 38 comments
Closed

[Discussion] IntelliJ and Yarn 2 integration #499

segrey opened this issue Sep 30, 2019 · 38 comments
Labels
discussion Discussion about the project

Comments

@segrey
Copy link
Contributor

segrey commented Sep 30, 2019

Opening this issue to discuss ways to integrate IntelliJ/WebStorm and Yarn 2 (with PnP) (https://youtrack.jetbrains.com/issue/WEB-35034). // cc @arcanis

Some questions regarding initial integration:

  1. Is possible to always generate resolution mappings as a JSON file under .pnp/ folder? That would allow IDE to not evaluate .pnp.js file and simplify obtaining whole dependency tree. If it's impossible right now, what do you think about adding it in future versions?

  2. IDE needs to index dependencies to provide coding assistance. Tried https://github.com/yarnpkg/pnp-sample-app (with yarn --version: 2.0.0-rc.6). For example, I have the following output:

require('./.pnp.js').resolveToUnqualified('jest', process.cwd() + '/')
'/home/segrey/examples/pnp-sample-app/.yarn/cache/jest-npm-23.5.0-bc11a2873d.zip/node_modules/jest'

Am I right that the tarballs downloaded from the registry are not unpacked anymore in yarn 2? Couldn't find such info at https://next.yarnpkg.com/features/pnp.

  1. Is there a way to access a particular file in a dependency? Previously, IDE could run /path/to/node <node options> /path/to/app/node_modules/gulp/bin/gulp.js <gulp options>. Should it be replaced with yarn run <binary name exposed by a dependency=gulp>?
    According to yarn run -h=1, it says that yarn run gulp will try to run a script with the same name first. It feels a bit unsafe to run an arbitrary command. Probably, not a big deal, however is it possible to ensure that a binary exposed by a local dependency is run?

  2. What's the recommended way to run a a local application (not a local dependency)?

  3. It might sound silly, but is there an option to create symlinks for each dependency to have node_modules with symlinks?

@arcanis
Copy link
Member

arcanis commented Oct 1, 2019

Hi Sergey!

Is possible to always generate resolution mappings as a JSON file under .pnp/ folder? That would allow IDE to not evaluate .pnp.js file and simplify obtaining whole dependency tree. If it's impossible right now, what do you think about adding it in future versions?

It's possible; projects can toggle off pnpEnableInlining and Yarn will generate a .pnp.data.json file ready to be consumed by the @yarnpkg/pnp package we provide. I have some documentation on how to use the package, but basically it's just:

import {hydratePnpFile} from '@yarnpkg/pnp';

hydratePnpFile(`${pathToProject}/.pnp.data.json`).then(pnp => {
  // Use `pnp` as an instance of the PnP API, cf
  // https://next.yarnpkg.com/advanced/pnpapi
});

Am I right that the tarballs downloaded from the registry are not unpacked anymore in yarn 2? Couldn't find such info at https://next.yarnpkg.com/features/pnp.

Yep exactly. I will add a section about this in the documentation, it should definitely be mentioned in at least a few places!

Previously, IDE could run /path/to/node <node options> /path/to/app/node_modules/gulp/bin/gulp.js <gulp options>. Should it be replaced with yarn run <binary name exposed by a dependency=gulp>?

Yep.

According to yarn run -h=1, it says that yarn run gulp will try to run a script with the same name first. It feels a bit unsafe to run an arbitrary command. Probably, not a big deal, however is it possible to ensure that a binary exposed by a local dependency is run?

Just to be sure we're on the same page: "script" is used here in the sense of "an entry of the scripts field from the package.json". So you will always use something that comes from the project, not from the local environment (which would break determinism).

It's actually pretty cool because it allows your users to configure exactly how gulp is meant to be executed if they wish to, or to wrap its execution. For example, the following script will transparently send all the gulp output into a log file:

{
  "scripts": {
    "gulp": "gulp \"$@\" | tee -a /tmp/gulp.log"
  }
}

What's the recommended way to run a a local application (not a local dependency)?

Do you mean something like yarn global? At the moment the closest we have is yarn dlx (which works pretty much like npx for this use case). There are some improvements we need to do on developer experience (for example caching the build artifacts, etc), but that's the closest we have.

It might sound silly, but is there an option to create symlinks for each dependency to have node_modules with symlinks?

Not silly! At the moment we don't have mechanisms to do that (especially since the packages are now kept within their archives).

@arcanis arcanis added the discussion Discussion about the project label Oct 1, 2019
@segrey
Copy link
Contributor Author

segrey commented Oct 1, 2019

Awesome, thanks!

Regarding resolution mappings as a JSON file, unfortunately, pnpEnableInlining is not a perfect option, because IDE cannot assume all yarn install are run with this option enabled. My initial though was to generate .pnp.data.json always (and probably consume it in .pnp.js). BTW, how to obtain @yarnpkg/pnp in this case? Seems IDE should bundle it, right? Maybe it's possible to just use .pnp.js API to obtain the dependency tree?
Honestly, initially, I was thinking about some public JSON API for .pnp.data.json, so tools can read the JSON file directly without any runtime dependencies (e.g. node, @yarnpkg/pnp or .pnp.js). Does it sound realistic?

Just to be sure we're on the same page: "script" is used here in the sense of "an entry of the scripts field from the package.json".

Correct. My concert is that, for instance{"scripts": {"jest":"echo hello"}} won't run Jest tests. Or, in your example, it's impossible to list all gulp tasks with the following command yarn run gulp --no-color --gulpfile /path/to/gulpfile --tasks-json --silent (IDE expects it will produce emit JSON to its standard output, but it will be written to a file).
BTW, is gulp supported in Yarn 2? Got the following output:

$ yarn run gulp 
[15:13:16] Local gulp not found in ~/samples/pnp/pnp-sample-app
[15:13:16] Try running: npm install gulp

Do you mean something like yarn global?

Not exactly, sorry for being unclear. I meant a simple scenario to run an application. It is done with node app.js in a non Yarn PnP environment. When running in Yarn PnP environment, the app starts, but fails when loading any declared (and installed) dependency, e.g. Error: Cannot find module 'react'. It works fine when running as node --require ./.pnp.js a.js. Is it the recommended way? Couldn't find it in https://next.yarnpkg.com/features/pnp. Maybe there are other docs I missed.

@arcanis
Copy link
Member

arcanis commented Oct 1, 2019

Honestly, initially, I was thinking about some public JSON API for .pnp.data.json, so tools can read the JSON file directly without any runtime dependencies (e.g. node, @yarnpkg/pnp or .pnp.js). Does it sound realistic?

Although it would be possible if Yarn's PnP implementation was the only one, I'm not a fan. There are various reasons for that:

  • In my mind, the PnP implementation shouldn't be restricted to Yarn's implementation. By offering a JS API rather than a JSON file, implementors have the ability to store their data differently if they wish to. For example, it would be trivial to implement an Haste-like resolution instead of the traditional one. In this sense, PnP meant to be a resolver API specification.

  • I don't think there's one format that will be good enough for everyone. Not for us (because we'll want to modify it from time to time), not for you (because you'll need it to be fixed in time), not for some other users which will have other requirements.

  • However, the PnP API is great in that it's a perfectly acceptable intermediary format: you can simply consume it and generate whatever file format you want! If you want JSON, you can. If you prefer YAML, sure. If you want to generate a Python file, well, be my guest.

Or, in your example, it's impossible to list all gulp tasks with the following command yarn run gulp --no-color --gulpfile /path/to/gulpfile --tasks-json --silent (IDE expects it will produce emit JSON to its standard output, but it will be written to a file)

In this particular case I used tee which forks the output and writes both to the file passed in parameter and the standard output (so both the file and the editor would have received JSON).

In general yes users can redefine the commands, but I'm not too concerned - it should be quite uncommon, and it's easy to surface special feedback when it happens by checking whether a script of the given name exists. I see as a feature more than a bug.

BTW, is gulp supported in Yarn 2? Got the following output:

I have to check - iirc Gulp is using a package that traverses up the node_modules hierarchy for some reason. But we should definitely fix that if it's still the case 🤔

Is it the recommended way? Couldn't find it in https://next.yarnpkg.com/features/pnp. Maybe there are other docs I missed.

When working from the CLI you have yarn node (we're following closely the Modules WG to eventually make this unnecessary), and when working from scripts (for example yarn build) you can just use node as you're used to:

{
  "scripts": {
    "start": "node ./index.js"
  }
}

@larixer
Copy link
Member

larixer commented Oct 1, 2019

Just to clarify the point of @arcanis about PnP API consumption and generating static file with the format you want:
https://next.yarnpkg.com/advanced/pnpapi#traversing-the-dependency-tree

In the example above the PnP tree is traversed, you can add serialization logic to this code and write PnP tree into a static file with the format you want.

@arcanis
Copy link
Member

arcanis commented Oct 1, 2019

I have to check - iirc Gulp is using a package that traverses up the node_modules hierarchy for some reason. But we should definitely fix that if it's still the case 🤔

I just double-checked; we're compatible with Gulp, the problem is that pnp-sample-app is very old now and uses package versions that didn't work well with us at the time.

For testing we now use dedicated E2E workflows that run every few hours against the latest releases of the projects we watch (Gulp isn't one of them at the moment, but it's probably still relevant enough to be worth tracking).

@segrey
Copy link
Contributor Author

segrey commented Oct 1, 2019

Great, thank you guys for the answers! It makes sense.

Although it would be possible if Yarn's PnP implementation was the only one, I'm not a fan.

Ah, looks like I didn't realize fully that PnP is not restricted to Yarn. Could PnP just use different sources (.pnp.data.json, Haste modules, etc.)? I don't know if this suggestion aligns well with PnP concept.
Regarding changing JSON file format, it shouldn't be a problem as long as the JSON file contains a version field, and there is a documentation for a new format.
Since, the PnP API stays, it can be used to generate a new file format (or .pnp.data.json can be consumed directly for that).
I understand that exposing .pnp.data.json file format implies more work on your side. Also, it's clear that it won't be implemented in the near future. So, anyway, the integration will use the generated .pnp.js file API.

I used tee which forks the output and writes both to the file passed in parameter and the standard output

Oops, right you are! :) In general, redefining commands is usually needed to specify some custom options. Then, if IDE passes the same options, it's unclear what will happen (the first wins, the latter wins or an error is thrown). I will let you know once I have a particular case.

I just double-checked; we're compatible with Gulp

Confirming, it works fine with gulp@4. Seems the problem is with gulp@3.

When working from the CLI you have yarn node

Thanks, didn't know about it. It did the trick! Couldn't find it in https://yarnpkg.com/en/docs/cli/. Seems it was added to Yarn 2.
Seems it doesn't support --scripts-prepend-node-path option, as a result IDE cannot ensure that the specified Node.js interpreter is used to run a user's script. Is it legitimate to use node --require ./.pnp.js my-app.js?

@arcanis
Copy link
Member

arcanis commented Oct 1, 2019

Thanks, didn't know about it. It did the trick! Couldn't find it in https://yarnpkg.com/en/docs/cli/. Seems it was added to Yarn 2.

It's also in Yarn 1, but I forgot to put it on its documentation 😅 The v2 one is here.

Seems it doesn't support --scripts-prepend-node-path option, as a result IDE cannot ensure that the specified Node.js interpreter is used to run a user's script. Is it legitimate to use node --require ./.pnp.js my-app.js?

Yarn (both v1 and v2) enforce child processes to always call the exact same Node and Yarn binaries as the ones that are running Yarn itself. So if you call Yarn with Node 8, for example, all subprocesses spawned with yarn node or yarn run will use Node 8 as well. To enforce the Node version you can just ensure it's in the PATH before calling Yarn.

In fact, --scripts-prepend-node-path was removed from Yarn in yarnpkg/yarn#7057 (Feb 2019).

@segrey
Copy link
Contributor Author

segrey commented Oct 1, 2019

@Vlasenko Thanks for the link! Tried to consume API in the following way:

// view-tree.js
console.log(require('pnpapi').getDependencyTreeRoots());

and got

$ yarn node view-tree.js 
/home/sergey/samples/pnp/react-app-example/view-tree.js:1
console.log(require('pnpapi').getDependencyTreeRoots());
                              ^

TypeError: require(...).getDependencyTreeRoots is not a function
    at Object.<anonymous> (/home/sergey/samples/pnp/react-app-example/view-tree.js:1:31)
    at Module._compile (internal/modules/cjs/loader.js:936:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:947:10)
    at Module.load (internal/modules/cjs/loader.js:790:32)
    at Function.module_1.default._load (/home/sergey/samples/pnp/react-app-example/.pnp.js:17965:20)
    at Function.Module.runMain (internal/modules/cjs/loader.js:999:10)
    at internal/main/run_main_module.js:17:11

What's wrong?

@segrey
Copy link
Contributor Author

segrey commented Oct 1, 2019

Yarn (both v1 and v2) enforce child processes to always call the exact same Node and Yarn binaries as the ones that are running Yarn itself.

Conducted the following experiment

// print-node-version.js
console.log(process.version);
$ /home/sergey/.nvm/versions/node/v12.9.0/bin/node /usr/share/yarn/bin/yarn.js node print-node-version.js
v12.9.0
$ /home/sergey/.nvm/versions/node/v12.4.0/bin/node /usr/share/yarn/bin/yarn.js node print-node-version.js
v12.9.0

Seems Yarn and the child process are run with different Node versions.

@arcanis
Copy link
Member

arcanis commented Oct 1, 2019

What's wrong?

The getDependencyTreeRoots function is fairly recent (#487). You might want to run the following commands to be sure to be on an up-to date release:

yarn set version from sources
yarn install

Seems Yarn and the child process are run with different Node versions.

I don't reproduce that, be it the v1 or v2 ... 🤔

@segrey
Copy link
Contributor Author

segrey commented Oct 1, 2019

Updating to an up-to date release doesn't help:

$ cat test.sh 
#!/bin/bash

set -x
set -e

mkdir new-project
cd new-project

echo '{}' > package.json
yarn set version from sources
yarn install
yarn node -e "require('pnpapi').getDependencyTreeRoots()"

$ rm -rf new-project/
$ ./test.sh 
+ set -e
+ mkdir new-project
+ cd new-project
+ echo '{}'
+ yarn set version from sources
➤ YN0000: Fetching the latest commits

HEAD is now at c5c6595 Improves the lock mechanism (#501)
Removing packages/yarnpkg-cli/bundles/
HEAD is now at c5c6595 Improves the lock mechanism (#501)

➤ YN0000: Building a fresh bundle

✓ Done building the CLI!
? Bundle path: /tmp/yarnpkg-sources/packages/yarnpkg-cli/bundles/yarn.js
? Bundle size: 6.97 MB
    → @yarnpkg/plugin-essentials
    → @yarnpkg/plugin-constraints
    → @yarnpkg/plugin-dlx
    → @yarnpkg/plugin-file
    → @yarnpkg/plugin-git
    → @yarnpkg/plugin-github
    → @yarnpkg/plugin-http
    → @yarnpkg/plugin-init
    → @yarnpkg/plugin-link
    → @yarnpkg/plugin-npm
    → @yarnpkg/plugin-npm-cli
    → @yarnpkg/plugin-pack
    → @yarnpkg/plugin-pnp
    → @yarnpkg/plugin-version

➤ YN0000: Saving the new release in .yarn/releases/yarn-sources.js
➤ YN0000: Done in 15.35s
+ yarn install
➤ YN0000: ┌ Resolution step
➤ YN0000: └ Completed in 0s
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed in 0s
➤ YN0000: ┌ Link step
➤ YN0000: └ Completed in 0s
➤ YN0000: Done in 0.01s
+ yarn node -e 'require('\''pnpapi'\'').getDependencyTreeRoots()'
[eval]:1
require('pnpapi').getDependencyTreeRoots()
                  ^

TypeError: require(...).getDependencyTreeRoots is not a function
    at [eval]:1:19
    at Script.runInThisContext (vm.js:126:20)
    at Object.runInThisContext (vm.js:316:38)
    at Object.<anonymous> ([eval]-wrapper:9:26)
    at Module._compile (internal/modules/cjs/loader.js:868:30)
    at evalScript (internal/process/execution.js:80:25)
    at internal/main/eval_string.js:23:3

@segrey
Copy link
Contributor Author

segrey commented Oct 1, 2019

@arcanis Never mind, it works now after removing ~/.yarnrc that was pointing to yarn v2 installed via $ yarn policies set-version v2

@segrey
Copy link
Contributor Author

segrey commented Oct 1, 2019

Opened #502 regarding running Yarn itself and child process with different Node versions.

@segrey
Copy link
Contributor Author

segrey commented Oct 4, 2019

IDE needs to index a dependency to provide coding assistance. Trying to fetch a local real path (inside a zip archive) for a package:

> require('./.pnp.js').resolveToUnqualified('react-dom', process.cwd() + '/')
'/home/sergey/samples/pnp/react-app-example/.yarn/virtual/react-dom-virtual-68ebc8ec11/0/cache/react-dom-npm-16.10.1-7c79a4c72f.zip/node_modules/react-dom'

The problem is that /home/sergey/samples/pnp/react-app-example/.yarn/virtual/react-dom-virtual-68ebc8ec11/0/cache/react-dom-npm-16.10.1-7c79a4c72f.zip doesn't exists. However, /home/sergey/samples/pnp/react-app-example/.yarn/cache/react-dom-npm-16.10.1-7c79a4c72f.zip does exist.

@arcanis Is it possible to fetch a real path on file system for a package using PnP API?

@arcanis
Copy link
Member

arcanis commented Oct 4, 2019

Hm not using the API, although you can do it like this:

const {statSync} = require(`fs`);

const path = process.argv[2];
const parts = path.split(`/`);

let current = ``;
let base = null;

for (let t = 0; t < parts.length; ++t) {
    current += `/${parts[t]}`;
    if (statSync(current).isFile()) {
        base = current;
        break;
    }
}

console.log(base);

@segrey
Copy link
Contributor Author

segrey commented Oct 4, 2019

IIUC, this code returns /path/to/.yarn/my-package.1.2.3-7c79a4c72f.zip from /path/to/.yarn/my-package.1.2.3-7c79a4c72f.zip/node_modules/my-package. However, in my case, .../.yarn/virtual/ doesn't exists, so the function won't find base.

@segrey
Copy link
Contributor Author

segrey commented Oct 4, 2019

Seems like IDE needs to repeat VirtualFS.mapToBase to map virtual/ path entry. Or is there a simpler/better way to get real file path?

@arcanis
Copy link
Member

arcanis commented Oct 4, 2019

Right, virtual paths mess with that approach 🤔

Right now I think replicating the VirtualFS logic is the simplest way to unblock this. We could fix that for zip archives (by mounting the virtual paths into the archives themselves, so you'd encounter the archive before the virtual folder), but that wouldn't work for workspaces.

@segrey
Copy link
Contributor Author

segrey commented Oct 8, 2019

Replicating the VirtualFS logic is done and it works. Would be great to get rid of this replication in future, because it feels a bit unsafe. Are there any chances to hit wrong .zip archive by simply removing virtual/[^/]*/[^/]*/[^/]*/ from location?

@arcanis
Copy link
Member

arcanis commented Oct 8, 2019

Simply removing this part isn't enough, in particular because the archives can be located outside of the .yarn directory (for example if the user defined their own cache folder). The virtual path must be interpreted as such:

/.../virtual/([^/]+)/([0-9]+)/(.*)
  • The first match (a random hash) can be ignored.
  • The second match (a number) is the relative number of parent directory that we must go up before resolving the rest of the path (the third match), starting from right before the virtual folder.

So for example:

virtual real
/foo/bar/virtual/abcdef/0/hello /foo/bar/hello
/foo/bar/virtual/abcdef/1/hello /foo/hello
/foo/bar/virtual/abcdef/2/hello /hello

Note that I understand your concerns - I'll try to think of a long-term solution to avoid having to hardcode this logic in the code.

@segrey
Copy link
Contributor Author

segrey commented Oct 15, 2019

@arcanis Small questions:

  • Is it possible to fetch installed dependency versions through PnP API, or it's better to read version property in package.json of the installed dependency?
  • Are .yarn/ folder and .pnp.js file located in the same parent folder always?

@arcanis
Copy link
Member

arcanis commented Oct 15, 2019

Is it possible to fetch installed dependency versions through PnP API, or it's better to read version property in package.json of the installed dependency?

Better to read the version from the package files. The PnP API itself only deals with packages dependencies and locations, not their metadata per se.

Are .yarn/ folder and .pnp.js file located in the same parent folder always?

By default yes, but the .yarn folder is configurable (for example through cacheFolder).

@segrey
Copy link
Contributor Author

segrey commented Oct 15, 2019

Thanks, I see. Then, maybe .yarnrc and .pnp.js files are located in the same parent folder always?

@arcanis
Copy link
Member

arcanis commented Oct 15, 2019

Often, but there might be other .yarnrc.yml files elsewhere (for example in $HOME). What are you trying to achieve exactly? Locate the project root? Wouldn't .git work?

@segrey
Copy link
Contributor Author

segrey commented Oct 15, 2019

I'm trying to locate all yarn internal files inside project to exclude them from indexing, so symbols defined in these files are not suggested in user's code. Currently, .pnp.js files and .yarn/ folders are excluded.

@arcanis
Copy link
Member

arcanis commented Oct 15, 2019

Oh I see - for this purpose, .pnp.js and .yarn (and potentially .pnp.meta.json) are indeed the folders you'd want to exclude 👍

@segrey
Copy link
Contributor Author

segrey commented Oct 15, 2019

Alright, thanks! JSON files can be kept as is (not excluded), because they are not indexed. BTW, curious, is .pnp.meta.json the same as .pnp.data.json generated by enabled pnpEnableInlining?

@arcanis
Copy link
Member

arcanis commented Oct 15, 2019

Yep! I misremembered the name, it's .pnp.data.json indeed 😅

@arcanis
Copy link
Member

arcanis commented Oct 28, 2019

Fyi, new Yarn builds (not yet released but you can test them with yarn set version from sources) will expose a new function called resolveVirtual which will return the resolved path. This should make it possible to implement the mechanism I described in #499 (comment) 🙂

@segrey
Copy link
Contributor Author

segrey commented Oct 30, 2019

@arcanis Thanks! It works fine. What does mean the versions object in the function's documentation?

you first must check that the versions object contains a valid resolveVirtual property

A general question regarding resolveToUnqualified: am I right that both require(dependencyName) and require(require('pnpapi').resolveToUnqualified(dependencyName, __dirname + '/')) will produce the same result? (Assuming both require calls are in the same file.)

And a related question: am I right that PackageInformation.location and require('pnpapi').resolveToUnqualified(dependencyName, <requester=package.json directory>) can be require'd legitimately? Is it some kind of absolute path for require? Like in npm, an absolute path to package directory under node_modules can be required, e.g. require('/home/user/my_project/node_modules/lodash/').

Couldn't find the exact answers in the docs, but I suspect the answers could be concluded.

@arcanis
Copy link
Member

arcanis commented Oct 31, 2019

Thanks! It works fine. What does mean the versions object in the function's documentation?

It's the VERSION dict of the PnP API - I should add a link.

A general question regarding resolveToUnqualified: am I right that both require(dependencyName) and require(require('pnpapi').resolveToUnqualified(dependencyName, __dirname + '/')) will produce the same result? (Assuming both require calls are in the same file.)

Yep! The resolveToUnqualified will resolve everything but the extension (and main.js etc), and when passed to require Node will run the remaining part of the resolution.

And a related question: am I right that PackageInformation.location and require('pnpapi').resolveToUnqualified(dependencyName, <requester=package.json directory>) can be require'd legitimately? Is it some kind of absolute path for require?

Yep, the return of resolveToUnqualified for a bare identifier is safe to forward to require (assuming that the target package has either an index.js or a valid main definition), and the same is true for packageLocation.

Btw, do you have a rough idea when WebStorm will release a beta with PnP support? I have some engineers at my company that are pretty excited about it! 😄

@segrey
Copy link
Contributor Author

segrey commented Oct 31, 2019

Cool, thanks! I'm detecting resolveVirtual as simple as typeof require('pnpapi').resolveVirtual === 'function'. Is it enough, or checking pnp.VERSIONS.resolveVirtual might be helpful sometimes?

I hope the next WebStorm 2019.3 EAP (planned for the next week) will have more or less ready to use PnP support. Also, https://youtrack.jetbrains.com/issue/WEB-35034 will be updated, you will receive a notification! ;)

@arcanis
Copy link
Member

arcanis commented Oct 31, 2019

Eheh I already lurk around this issue 😄 Awesome, thanks!

I'm detecting resolveVirtual as simple as typeof require('pnpapi').resolveVirtual === 'function'. Is it enough, or checking pnp.VERSIONS.resolveVirtual might be helpful sometimes?

That's fine - the version number may be useful for backward compatibility (for example later on we might add options, in which case you'll be able to check the version field to know whether they are supported by the API you work with), but for the first version a good old typeof is enough 🙂

@segrey
Copy link
Contributor Author

segrey commented Nov 1, 2019

@arcanis Do you know if it's possible to run ESLint in a default create-react-app project (with deps installed by Yarn PnP)? Currently, it seems to be not working (https://youtrack.jetbrains.com/issue/WEB-42177).

@arcanis
Copy link
Member

arcanis commented Nov 1, 2019

I think this problem is because the default Create-React-App ESLint configuration relies on hoisting to work properly. The eslint-config-react-app package isn't a dependency of the top-level (only of react-scripts), so it shouldn't be possible to require it.

Imo the best solution would come from CRA, by exposing a new file in react-scripts called eslint-config.js and changing the default package.json configuration to something akin to this:

{
  "eslintConfig": {
    "extends": "react-scripts/eslint-config"
  }
}

Unfortunately the problem is that ESLint currently always prefixes the package name with eslint-config, which makes the require fail too (because eslint-config-react-scripts isn't a dependency too)... 🤔

The second best solution is to simply add eslint-config-react-app to your dependencies, but that gets a bit hairy because it has many peer dependencies 😔

cc @iansu - do you have another idea?

@segrey
Copy link
Contributor Author

segrey commented Nov 1, 2019

Thanks, interesting. IIUC, both solutions assume that eslint is a dependency, right? Regarding the first solution, while ESLint prefixes the package name with eslint-config, your solution can be altered a bit to add eslint-config-react-scripts as a dependency, and default package.json configuration would change to

{
  "dependencies": {
    "eslint": "*",
    "eslint-config-react-scripts": "*",
    "... other deps ..."
  },
  "eslintConfig": {
    "extends": "react-scripts"
  }
}

Will it work? Is there an issue to track ESLint support in create-react-app? Thanks!

@segrey
Copy link
Contributor Author

segrey commented Nov 7, 2019

WebStorm 2019.3 EAP build 193.5096.13 brings supports for Yarn PnP. 🎉
@arcanis @larixer Thank you guys for the help!

@prewk
Copy link

prewk commented Mar 20, 2024

Reviving this old discussion with a question to its contributors: Have you encountered a problem where IntelliJ (and VS Code!) are constantly re-indexing stuff? I can only tab out of WebStorm (which - on changes - will save unsaved files) and when I come back it'll reindex. Every time.

This is with yarn berry zero-install and enableGlobalCache: false. It doesn't happen on classic yarn.

I've reinstalled to no avail, and this happens on multiple computers.

Also, when I monitor my project's files with chokidar-cli, i see that it's constantly changing/touching the cache files, without me making any changes. This stops immediately when I close the IDE. Excerpt:

$ chokidar "*.*" "**/*.*" -c "echo '{path}'"

.....
change:.yarn/cache/@storybook-angular-npm-7.5.3-cb80502f1a-fd51664361.zip
change:.yarn/cache/@testing-library-angular-npm-14.2.0-e1624ada75-e41e72edf7.zip
change:.yarn/cache/@testing-library-angular-npm-14.2.0-e1624ada75-e41e72edf7.zip
change:.yarn/cache/@schematics-angular-npm-17.2.3-1576f57e2c-a350fbbd91.zip
.yarn/cache/@schematics-angular-npm-17.2.3-1576f57e2c-a350fbbd91.zip
change:.nx/cache/project-graph.json
change:.nx/cache/file-map.json
.nx/cache/file-map.json

I know this may also belong in JetBrain's issue trackers but I think more relevant people will see it here.

edit: In classic rubberduck manner, the fact that I posted it here made me think about it in new ways and I found the culprit: The official NX plugin for WebStorm (and, I would assume, for VSCode). It seems to be having trouble with PNP. Disabling it fixed the re-indexing issue.

Hope this will help someone! nrwl/nx-console#2062

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion about the project
Projects
None yet
Development

No branches or pull requests

4 participants