[DevTools Bug]: With yarn@3.2.1 and PnP and react-devtools@4.24.7, yarn react-devtools fails to start

[wegry]

Website or app

https://github.com/wegry/pnp-and-react-devtools

Repro steps

1. In project using yarn@3.2.1 and PnP (yarn set version berry)
2. yarn add -D react-devtools
3. yarn react-devtools
4. See error

Uncaught Exception:
TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
at validateString (internal/validators.js:120:11)
at Object.normalize (path.js:1005:5)
at contains (/Users/zw/Project/.pnp.cjs:40645:18)
at Object.ppath.contains (/Users/zw/Project/.pnp.cjs:40659:32)
at isPathIgnored (/Users/zw/Project/.pnp.cjs:49165:27)
at findPackageLocator (/Users/zw/Project/.pnp.cjs:49235:9)
at Object.findPackageLocator (/Users/zw/Project/.pnp.cjs:49527:14)
at findApiPathFor (/Users/zw/Project/.pnp.cjs:49618:41)
at Object.getApiPathFromParent (/Users/zw/Project/.pnp.cjs:49669:36)
at require$$0.Module._load (/Users/zw/Project/.pnp.cjs:48576:40)

How often does this bug happen?

Every time

DevTools package (automated)

No response

DevTools version (automated)

No response

Error message (automated)

No response

Error call stack (automated)

No response

Error component stack (automated)

No response

GitHub query string (automated)

No response

[mondaychen]

I'm not familiar with PnP, but looks like when react-devtools is trying to locate its file app.js via require.resolve('./app'), it points to a location within a zip file react-devtools-npm-4.24.7-03a21e386b-297f6a2ba3.zip/node_modules/react-devtools/app.js, thus does not work.
This is probably a bug on PnP side? If they change how require works, they should change how require.resolve works as well.

[wegry]

I've opened a discussion over at yarnpkg/berry#4536 about this issue to see if someone can round out my understanding.

[arcanis]

This is probably a bug on PnP side? If they change how require works, they should change how require.resolve works as well.

It does, that's why you get a path within a zip file.

This special virtual path can be loaded when the PnP runtime is live; we configure it via NODE_OPTIONS so usually it's not a problem, but perhaps Electron or react-devtools ignore this environment variable or override it.

[mondaychen]

@arcanis Thanks for the information. react-devtools is running electron via spawn https://github.com/facebook/react/blob/main/packages/react-devtools/bin.js#L32
Maybe something got lost in the middle? I'll test it when I have time later today

Edit: I made sure that NODE_OPTIONS is in env

NODE_OPTIONS: '--require /path/to/pnp-and-react-devtools/.pnp.cjs'

But it's still not working. Maybe electron ignores it somehow.

[m-yasir]

@mondaychen In reference to your comment on electron/electron#34470

I believe I understand the problem and that there's a change needed on react-devtools.

As evident in https://github.com/facebook/react/blob/main/packages/react-devtools/bin.js#L32 and from what I understand, there are no environment variables being set through cross-spawn to pass on to electron's process. Something like this would be needed for it to work:

const electron = require('electron');
const result = spawn.sync(electron, [require.resolve('./app')].concat(argv), {
  stdio: 'ignore',
  env: { NODE_OPTIONS: ... }
});

Would you mind if I take this issue and open a pull request, if it works? 😃 The only question I have right now is whether the node process for devtools will have NODE_OPTIONS available within its environment (or argv). If yes, and assuming the NODE_OPTIONS are sanitary by default for devtools, passing process.env.NODE_OPTIONS should fix it.

References

Note: cross-spawn seems to be using nodejs' child_process.spawn for processes.
https://github.com/moxystudio/node-cross-spawn/blob/5d843849e1ed434b7030e0aa49281c2bf4ad2e71/index.js#L12
https://nodejs.org/api/child_process.html#child_processspawncommand-args-options

Edit: According to @arcanis's comment, and upon investigation, cross-spawn/spawn do seem to preserve env.

[arcanis]

Looking at electron/electron#13402, it seems Electron purposefully ignores NODE_OPTIONS, so it cannot access files located within zip archives 😕

Something you can try is to run yarn unplug react-devtools, but I'm not 100% sure it will work fine, as the dependencies won't be available via the node_modules folder that the default Electron would expect.

[m-yasir]

@arcanis I believe that would be true for packaged apps, but devtools seems to be spinning up a non-packaged electron process, --require should not be ignored within NODE_OPTIONS, at least according to the docs.

See https://www.electronjs.org/docs/latest/api/environment-variables#node_options

[arcanis]

Interesting - afaik cross-spawn (and spawn) default to preserve the env, so if it's not explicitly overridden I'd expect it to work then 🤔

[mondaychen]

@m-yasir we would greatly appreciate a PR! But I believe, like @arcanis mentioned, spawn preserves existing env by default.

[m-yasir]

@mondaychen That seems to be correct, bad speculation on my end. If environment is preserved then this is a non-issue. In any case, I think this would need further investigation and discussion before it gets to PR stage (if it's even needed).

This definitely looks like an interesting behaviour. I'll follow this issue in any case and investigate if I can 😃

[mnajafiy]

This is not only limited to react-devtools. It seems to be something in Electron and package installation with yarn 3.2.x
This is my package.json and I still get the same error when I do yarn start

{
   ...
  "main": "public/electron.js",
  "homepage": "./",
  "dependencies": {
    "@craco/craco": "^7.0.0-alpha.3",
    "@types/node": "^18.6.3",
    "autoprefixer": "^10.4.7",
    "concurrently": "^7.3.0",
    "cosmiconfig": "^7.0.1",
    "cosmiconfig-typescript-loader": "^3.1.1",
    "cross-env": "^7.0.3",
    "global": "^4.4.0",
    "postcss": "^8.4.14",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "react-scripts": "latest",
    "tailwindcss": "^3.1.7",
    "tailwindcss-cli": "^0.1.2",
    "ts-node": "^10.9.1",
    "typescript": "^4.7.4",
    "wait-on": "^6.0.1",
    "web-vitals": "^2.1.0"
  },
  "scripts": {
    "start": "concurrently \"cross-env BROWSER=none cross-env TAILWIND_MODE=watch craco start\" \"wait-on http://localhost:3000 && electron .\"",
    "build": "craco build",
    "test": "craco test",
    "eslint": "eslint src/ public/ --ext .jsx,.js,.ts,.tsx --max-warnings 0",
    "gen-lic": "yarn licenses list --ignore-platform > oss/oss-licenses.txt",
    "gen-lic-disclaimer": "yarn licenses generate-disclaimer --ignore-platform  > oss/oss-licenses-disclaimer.txt"
  },
  "eslintConfig": {
    "extends": []
  },
  "browserslist": {
    "production": [
      ">0.2%",
      "not dead",
      "not op_mini all"
    ],
    "development": [
      "last 1 chrome version",
      "last 1 firefox version",
      "last 1 safari version"
    ]
  },
  "devDependencies": {
    "@babel/core": "^7.18.9",
    "@babel/plugin-syntax-flow": "^7.18.6",
    "@babel/plugin-transform-react-jsx": "^7.18.6",
    "@testing-library/dom": "^8.16.0",
    "@testing-library/jest-dom": "^5.16.4",
    "@testing-library/react": "^13.3.0",
    "@testing-library/user-event": "^14.3.0",
    "electron": "^19.0.10",
    "eslint": "^8.20.0",
    "eslint-config-prettier": "^8.5.0",
    "eslint-config-standard": "^17.0.0",
    "eslint-plugin-electron": "^7.0.0",
    "eslint-plugin-flowtype": "^8.0.3",
    "eslint-plugin-import": "^2.26.0",
    "eslint-plugin-n": "^15.2.4",
    "eslint-plugin-node": "^11.1.0",
    "eslint-plugin-prettier": "^4.2.1",
    "eslint-plugin-promise": "^6.0.0",
    "eslint-plugin-react": "^7.30.1",
    "eslint-plugin-react-hooks": "^4.6.0",
    "eslint-plugin-simple-import-sort": "^7.0.0",
    "prettier": "^2.7.1",
    "react-dev-utils": "^12.0.1",
    "typescript-eslint": "^0.0.1-alpha.0",
    "webpack": "^5.74.0"
  },
  "packageManager": "yarn@3.2.2"
}