`@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](https://issuehunt.io/u/voxpelli) has been rewarded. ### Backers (Total: $54.00) - [ sindresorhus](https://issuehunt.io/u/sindresorhus) ($54.00) ### Submitted pull Requests - [#624 Fix installing XO on npm 6 by bundling TS-dependent dependencies to avoid hoisting](https://issuehunt.io/r/xojs/xo/pull/624) --- ### Tips - Checkout the [Issuehunt explorer](https://issuehunt.io/r/xojs/xo/) to discover more funded issues. - Need some help from other developers? [Add your repositories](https://issuehunt.io/r/new) 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 https://github.com/xojs/eslint-config-xo-typescript/commit/2e16d51a8cdcd1e34a827fa8e782075888590062 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: https://github.com/typescript-eslint/typescript-eslint/issues/828

[issuehunt-oss[bot]]

@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: https://github.com/SamVerschueren/tsd-typescript/pull/3 And an issue in tsd about it: https://github.com/SamVerschueren/tsd/issues/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[bot]]

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

• :moneybag: Total deposit: $54.00
• :tada: Repository reward(0%): $0.00
• :wrench: Service fee(10%): $5.40