@typescript-eslint/parser breaks XO

[sindresorhus]

Error: Failed to load parser '/home/runner/work/p-cancelable/p-cancelable/node_modules/@typescript-eslint/parser/dist/index.js' declared in 'BaseConfig': Cannot find module 'typescript'
9
Require stack:
10
- /home/runner/work/p-cancelable/p-cancelable/node_modules/@typescript-eslint/typescript-estree/dist/parser.js
11
- /home/runner/work/p-cancelable/p-cancelable/node_modules/@typescript-eslint/typescript-estree/dist/index.js
12
- /home/runner/work/p-cancelable/p-cancelable/node_modules/@typescript-eslint/parser/dist/parser.js
13
- /home/runner/work/p-cancelable/p-cancelable/node_modules/@typescript-eslint/parser/dist/index.js
14
- /home/runner/work/p-cancelable/p-cancelable/node_modules/@eslint/eslintrc/lib/config-array-factory.js
15
- /home/runner/work/p-cancelable/p-cancelable/node_modules/@eslint/eslintrc/lib/index.js
16
- /home/runner/work/p-cancelable/p-cancelable/node_modules/eslint/lib/cli-engine/cli-engine.js
17
- /home/runner/work/p-cancelable/p-cancelable/node_modules/eslint/lib/cli-engine/index.js
18
- /home/runner/work/p-cancelable/p-cancelable/node_modules/eslint/lib/api.js
19
- /home/runner/work/p-cancelable/p-cancelable/node_modules/xo/index.js
20
- /home/runner/work/p-cancelable/p-cancelable/node_modules/xo/cli-main.js
21
- /home/runner/work/p-cancelable/p-cancelable/node_modules/xo/cli.js
22
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:885:15)
23
    at Function.Module._load (internal/modules/cjs/loader.js:730:27)
24
    at Module.require (internal/modules/cjs/loader.js:957:19)
25
    at require (internal/modules/cjs/helpers.js:88:18)
26
    at Object.<anonymous> (/home/runner/work/p-cancelable/p-cancelable/node_modules/@typescript-eslint/typescript-estree/dist/parser.js:30:25)
27
    at Module._compile (internal/modules/cjs/loader.js:1068:30)
28
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
29
    at Module.load (internal/modules/cjs/loader.js:933:32)
30
    at Function.Module._load (internal/modules/cjs/loader.js:774:14)
31
    at Module.require (internal/modules/cjs/loader.js:957:19)
32
npm ERR! Test failed.  See above for more details.

https://github.com/sindresorhus/p-cancelable/runs/2711102803?check_suite_focus=true

---

IssueHunt Summary

voxpelli has been rewarded.

Backers (Total: $54.00)

• sindresorhus ($54.00)

Submitted pull Requests

• #624 Fix installing XO on npm 6 by bundling TS-dependent dependencies to avoid hoisting

---

Tips

• Checkout the Issuehunt explorer to discover more funded issues.
• Need some help from other developers? Add your repositories on IssueHunt to raise funds.

[sindresorhus]

// @Richienb I think this is related to recent changes as it only happened when upgrading XO.

[Richienb]

I wonder why this problem only appears on Node.js 12 and 14 but not on 16. Perhaps a different module resolution algorithm is actually able to find the typescript module or npm is installing it in a slightly different path.

[sindresorhus]

I think I have figured it out. Node.js 16 passes because it has npm 7 which install peer dependencies by default. I'm hoping xojs/eslint-config-xo-typescript@2e16d51 will fix the issue on other Node.js versions.

[sindresorhus]

Did not work...

[sindresorhus]

@Richienb We need to find a workaround for now. npm 6 will be with us for as long as Node.js 14, which means years.

One idea I had was to add a postinstall script to XO that installs typescript into node_modules/@typescript-eslint/parser

---

Related issue: typescript-eslint/typescript-eslint#828

[issuehunt-oss]

@sindresorhus has funded $54.00 to this issue.

---

• Submit pull request via IssueHunt to receive this reward.
• Want to contribute? Chip in to this issue via IssueHunt.
• Checkout the IssueHunt Issue Explorer to see more funded issues.
• Need help from developers? Add your repository on IssueHunt to raise funds.

[voxpelli]

I think a solution to this problem would be to ship xo with a npm-shrinkwrap.json as it would then work like a package-lock.json during installation and ensure that everything gets installed as intended: https://docs.npmjs.com/cli/v7/configuring-npm/package-lock-json#package-lockjson-vs-npm-shrinkwrapjson

[voxpelli]

I dug waaaaaaay too deep into this and can now say why this error happens:

The part of npm 6.x that's responsible for deduping the three is earliestInstallable().

In that function there are a couple of checks to see if a module can be added earlier, and one of those checks are a check for conflicting binaries.

It bails on typescript for p-cancelable and the reason is: tsd

Since tsd has already hoisted @tsd/typescript (I believe npm processes dependencies in alphabetic order) it blocks the hoisting of typescript as they both (one being the fork of the other) exposes the same binaries.

I removed tsd from p-cancelable and did a fresh install and verified that typescript then gets correctly hoisted on npm 6.x.

There is a PR to solve this in @tsd/typescript: tsdjs/typescript#3 And an issue in tsd about it: tsdjs/tsd#122

If that PR gets merged and released, then as soon as p-cancelable picks up that new release, then tests will start working on with npm 6 again.

Though: Especially considering that xo, due to being pretty much last in the alphabet, is doomed to pretty much always be processed last, any other module before it that contains a binary conflict with typescript will stop typescript from being hoisted while the modules that requires typescript will still get hoisted.

The solutions to that is as said in previous comment: npm-shrinkwrap.json Or add them as bundledDependencies, though the latter will actually bundle those dependencies within the xo bundle on npm, creating quite a bit larger bundle for xo

[Richienb]

Since this isn't a browser-based tool, it could probably work to add them to bundledDependencies since size doesn't matter as much.

[sindresorhus]

@voxpelli That is some incredible investigation. Thanks for looking into this. 🙌

[sindresorhus]

I think we should use bundleDependencies. That way we are safe from this happening in the future. We can drop the bundleDependencies one day when we target Node.js 16.

[voxpelli]

@sindresorhus I have created a PR that should make this work 👍

[issuehunt-oss]

@sindresorhus has rewarded $48.60 to @voxpelli. See it on IssueHunt

• 💰 Total deposit: $54.00
• 🎉 Repository reward(0%): $0.00
• 🔧 Service fee(10%): $5.40