rpt2 does not resolve pnpm symlinked dependencies

[davelsan]

What happens and why it is wrong

When trying to build a project installed with pnpm, rpt2 fails to resolve the created symlinked dependencies in node_modules (non-flat).

Setting check: false is (arguably) a workaround.

Related issues: #188, #214

Reproduction

Repository: https://github.com/davelsan/rpt2-pnpm-symlinks
Config: https://github.com/davelsan/rpt2-pnpm-symlinks/blob/master/rollup.config.js

The provided repro is a minimal vue3 plugin that imports the App interface from vue in index.ts

import  { App } from 'vue'

This import causes the following error on build:

 [!] (plugin rpt2) Error: semantic error TS2305: Module '"../node_modules/vue/dist/vue"' has no exported member 'App'.

Setting the check option to false in rollup.config.js eliminates the error.

Versions

• typescript: 4.2.3
• rollup: 2.41.5
• rollup-plugin-typescript2: 0.30.0

rollup.config.js

import path    from 'path';
import rpt2    from 'rollup-plugin-typescript2';
import pkgJson from './package.json';

export default {

  input: './src/index.ts',

  external: ['vue'],

  plugins: [
    rpt2({
      check: true,
      tsconfig: path.resolve(__dirname, 'tsconfig.json'),
    })
  ],

  output: [
    {
      file: pkgJson.browser, // dist/index.esm.js
      format: 'es',
      sourcemap: true,
    },
  ],
}

tsconfig.json

A snippet with the relevant module resolution options only.

{
  "compilerOptions": {
    "target": "esnext",
    "module": "esnext",
    "moduleResolution": "node",
    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true,
  }
}

package.json

A snippet with the relevant dependencies only.

{
  "peerDependencies": {
    "vue": "^3.0.7"
  },
  "devDependencies": {
    "rollup": "^2.41.5",
    "rollup-plugin-typescript2": "^0.30.0",
    "typescript": "^4.2.3",
    "vue": "^3.0.7"
  }
}

[davelsan]

Update

According to this comment in the pnpm repo, there was a similar problem caused by the rollup-plugin-commonjs plugin (note that I'm not using it in the repro).

However, none of the provided solutions will work in this case.

// include the actual pnpm registry path
commonjs({ include: '../../node_modules/.pnpm/registry.npmjs.org/**' })

// or set the following option in rollup.config.js
preserveSymlinks: true

[davelsan]

I've been looking at the code and I'm mystified. Leaving more info here for myself and other passing-by folks:

• In theory resolve already supports symlinks (preserveSymlinks doesn't work properly from root package browserify/resolve#196 was fixed in [Fix] preserveSymlinks: false ensure that files are realpathed browserify/resolve#197)

• This is also supported in @rollup/plugin-resolve via the rollup option preserveSymlinks. Though it seems to use a different resolve algorithm.
  https://github.com/rollup/plugins/blob/9ad41a0760b4eeed9fd26d2da9b5122aa8a351e5/packages/node-resolve/src/index.js#L148-L156

• This plugin does use resolve.sync if rollupCommonJSResolveHack is set to true. But it does not pass an options object.

rollup-plugin-typescript2/src/index.ts

Lines 160 to 162 in 428140b

	const resolved = pluginOptions.rollupCommonJSResolveHack
	? resolve.sync(result.resolvedModule.resolvedFileName)
	: result.resolvedModule.resolvedFileName;

Based on the above, I cannot see any combination of options and/or plugins that will correctly resolve pnpm symlinks. Maybe I could try to implement it? It looks simple enough, which probably means it is more complicated than I can see 😅

[davelsan]

I'm starting to suspect this is not an issue with rpt2, despite what the error says, but rather a problem that's propagating upstream.

Closing until I have more information.

[theoparis]

Any updates on this? I'm trying to bundle a react app with rollup and the original @rollup/plugin-typescript gives debug failure errors, so I tried this.

I got the following error specifically:

[!] (plugin rpt2) Error: /media/theo/TheoData/Essentials/Projects/Programming/hypervpn/src/pages/home.tsx(1,10): semantic error TS2305: Module '"@chakra-ui/react"' has no exported member 'Button'.
src/pages/home.tsx
Error: /media/theo/TheoData/Essentials/Projects/Programming/hypervpn/src/pages/home.tsx(1,10): semantic error TS2305: Module '"@chakra-ui/react"' has no exported member 'Button'.
    at error (/media/theo/TheoData/Essentials/Projects/Programming/hypervpn/node_modules/.pnpm/[email protected]/node_modules/rollup/dist/shared/rollup.js:5279:30)

[theoparis]

Thing is, it works fine with the original ts plugin before I had debug failure errors, and running tsc by itself works fine as well.

[beetaa]

Thing is, it works fine with the original ts plugin before I had debug failure errors, and running tsc by itself works fine as well.

I am using pnpm as the package manager, same situation while rolling up a vue component. no problem when using @rollup/plugin-typescript.

[beetaa]

@davelsan @creepinson I think this issue should be opened again for further discussion. thank you.

[davelsan]

Hi @beetaa. Thanks for your interest in this issue. I have updated the original repro and I can confirm the issue is still there. I'm re-opening the discussion as per your request.

[ezolenko]

@davelsan

This plugin does use resolve.sync if rollupCommonJSResolveHack is set to true. But it does not pass an options object.
[...]
Based on the above, I cannot see any combination of options and/or plugins that will correctly resolve pnpm symlinks. Maybe I could try to implement it? It looks simple enough, which probably means it is more complicated than I can see 😅

So you are saying rpt2 should pass { preserveSymlinks: false } option to resolve?

[jackson-yyy]

[... shortened for readability ...]
So you are saying rpt2 should pass { preserveSymlinks: false } option to resolve?

I got the same problem in my project with pnpm.
I debugged line by line and found the problem is getAllReferences function returned a path the symlink, not the path of the real file or directory.

rollup-plugin-typescript2/src/tscache.ts

Lines 63 to 75 in 428140b

	export function getAllReferences(importer: string, snapshot: tsTypes.IScriptSnapshot | undefined, options: tsTypes.CompilerOptions)
	{
	if (!snapshot)
	return [];
	const info = tsModule.preProcessFile(snapshot.getText(0, snapshot.getLength()), true, true);
	return _.compact(_.concat(info.referencedFiles, info.importedFiles).map((reference) =>
	{
	const resolved = tsModule.nodeModuleNameResolver(reference.fileName, importer, options, tsModule.sys);
	return resolved.resolvedModule ? resolved.resolvedModule.resolvedFileName : undefined;
	}));
	}

for examples,
consider this following directory

node_modules
├─.pnpm
|  ├─[email protected]
|  |  ├─node_modules
|  |  |   ├─@vue
|  |  |   ├─vue
packages
|  ├─demo
|  |   ├─node_modules
|  |   │  └─vue(symlink, real path should be `project_root/node_modules/.pnpm/[email protected]/node_modules/vue`)
|  |   ├─public
|  |   └─src

As you see, the real path of vue in demo should be project_root/node_modules/.pnpm/[email protected]/node_modules/vue
But after resolving by tsModule.nodeModuleNameResolver in function getAllReferences , it becomes project_root/packages/demo/node_modules/vue

[email protected] depends on @vue, which is not exist in project_root/packages/demo/node_modules, so it throw an error while resolving.

I wonder if there is any way to solve this problem.
Otherwise i can only use pnpm --shamefully-hoist to hoist all packages to root node_modules, which is similar to yarn/npm.😅

[mammadataei]

Any news?
I have the same issue

semantic error TS2305: Module '"vue"' has no exported member 'defineComponent'.

[RookieChen4]

@rollup/plugin-typescript solve this.

[kdoroszewicz]

rollup-plugin-ts works too and also exports compiled d.ts typings without much of a hassle.

[agilgur5]

Fixed the issue!

So I believe I've found a fix for this long-standing issue! See #332

Notes on root cause analysis / debugging

Red herring to look at resolveId hook etc

I've been looking at the code and I'm mystified. Leaving more info here for myself and other passing-by folks:

It was helpful to see this previous research, but this seemed to be a bit of a red herring... I'll explain shortly.

This issue doesn't list "plugin output with verbosity: 3" as requested in the issue template, but #330 did, and I confirmed the fix in its reproduction. I also took the reproduction here and added verbosity: 3 and confirmed it had very similar logs. I also confirmed (by locally editing rpt2 in node_modules) that the fix works for this reproduction as well.

Notably, the logs don't output anything about "resolving", so the bug here isn't in the resolveId hook at all. Which is why looking at resolve etc was a red herring.
That portion of the code also resolves directly with TS and with tsModule.sys as its host. As I found out later, ts.sys has a default implementation of realpath, which turns out to be the important missing piece here.

Error is in transform hook actually

It actually errors right after the "transpiling" log statement, which means the error is actually in the transform hook, specifically in getEmitOutput, which is implemented by TS and doesn't have host-specific code for it... But apparently it seems to rely host.realpath under-the-hood.

Implementing host.realpath fixes this

So I tried implementing host.realpath (as #332 shows), and then everything worked and the type-error disappeared!
The typings were generated correctly as well now (#330 had broken typings when using check: false).

TS docs on Compiler APIs leave a lot to be desired... 😕 😞

The TS docs are incredibly sparse when it comes to Compiler integrations such as this plugin. The Compiler API basically has two small docs/examples in the TS Wiki... and that's it. Notably, there is no mention of realpath what-so-ever in either of those.

Honestly the root cause of a lot of TS integration issues can come down to poor documentation of the Compiler API, which has been noted in lots of places.
As a result, this requires even more significant domain knowledge to figure out issues like this, as the docs aren't even helpful -- I had to literally read through the Compiler source code... and have had to do that a handful of times in the past too. That's obviously not ideal, though at least we can do something like that in open-source.

Basically you need to be a TS Compiler subject matter expert to figure out some of these things, and I'm not sure I would even call myself that despite being a regular contributor to the TS tooling ecosystem (compilers are huge after all!).

Note on Other Rollup TS plugins

@rollup/plugin-typescript solve this.

rollup-plugin-ts works too and also exports compiled d.ts typings without much of a hassle.

I'm honestly not sure how these plugins manage to get around this issue as neither of them implement host.realpath

I've mentioned this elsewhere, but why I decided to contribute here (and now I help maintain this repo) as opposed to rollup-plugin-ts is because this repo was significantly simpler to wrap my head around. rollup-plugin-ts has quite a lot of features -- which is great! But it also means that one actually needs even more domain knowledge of its structure to contribute.

@rollup/plugin-typescript historically wasn't well maintained (that's why this fork came into existence after all), but is quite a bit better maintained nowadays.

I believe all 3 plugins (including this one) have different approaches toward various problems, so unfortunately the code can differ quite a bit. And all 3 plugins have their own sets of bugs and features too 😕

Shared core tooling would be great!

I'd love for TS tooling to use more shared packages though where the approach is the same!

E.g. the host implementation and tsconfig parsing are two areas that could probably be de-duped; I'm personally working on this in my very limited free time (and notably I started contributing here during my time solo-maintaining TSDX, so already investing upstream). Shared packages and composition are especially useful for complicated and poorly documented subareas of TS (or any codebase that needs significant domain knowledge for that matter).

(one example of composition would be running rollup-plugin-dts on top of this plugin or @rollup/plugin-typescript vs. using rollup-plugin-ts's built-in custom transformer for declaration bundling. Both of these have their own issues / bugs too -- TS doing this itself would be the ideal, then the whole ecosystem benefits and can use it!)

Misc

So you are saying rpt2 should pass { preserveSymlinks: false } option to resolve?

false is actually the default of resolve, Rollup itself, TS itself, which all reflect how Node itself treats symlinks (i.e. require.resolve).
And that was a red herring anyway

I also think we may not need resolve as a dep at all since we can use built-in Node methods (resolve was originally created for browserify after all).

[agilgur5]

Red herring to look at resolveId hook etc

Relatedly (but different root cause), I investigated #295 more after solving this issue and stepped through the resolution logic therein.
I confirmed my suspicion that resolveId works correctly due to its direct usage of tsModule.sys in #295 (comment), where the "resolving" logs are output with the correctly resolved realpath instead of the one to the symlink.

So not just a red herring, the resolveId code actually correctly handles symlinks already.

[agilgur5]

Released in 0.32.0 🎉