module resolution logic doesn't match tsc

[leightonsmith]

The module resolution logic in ts-pnp doesn't match tsc. Noticed when trying to use lodash/fp in a typescript project.

The differences are:

• Folder masking - a folder will mask a file with the same name (ignoring file extension). In our case, lodash/fp masks the file that should be imported - lodash/fp/fp.d.ts
• typesVersion - typescript allows a package to specify different folders for files depending on the version of typescript.

See https://github.com/leightonsmith/ts-pnp-resolution-demo for more info, and reproduction steps.

I'm going to take a shot at fixing this.

The folder masking bit should be simple enough to address (if there's a file, prefer over a folder)

The typesVersion part will take more work, and is overlapping with functionality already present in typescript. Maybe the correct way to address both problems is to get typescript to handle it? At the moment, this plugin hands typescript the full path; maybe it'd work better to hand typescript the path to the package, and then let it handle it?

I'll keep you posted with progress.

[leightonsmith]

Update on what I'm up to. Note that I'm ramping up in the node ecosystem, so feel free to let me know if I've done something weird, or missed something obvious.

I took a few tries at fooling the typescript compiler in a similar way to what pnpify does, e.g. by patching the file functions (fileExists, readFile and so on), but I got enough odd errors to persuade me it's not sensible.

I'm now going to try the simpler (more sensble) approach, of just re-implementing the module lookup logic. My reading of the comment at https://github.com/microsoft/TypeScript/blob/master/src/compiler/types.ts#L5446 also makes me think that we're expected to implement all the logic ourselves.

[arcanis]

Hey! Thanks for digging into it. Sharing here what I posted on Discord:

hey @leightonsmith! something I didn't quite understand was the reason for those discrepancies

TS has builtin hooks for the resolution process - it takes a request and an issuer, and returns a path

ts-pnp simply implements a thin conversion that transforms the bare import (lodash) into an unqualified absolute import (/path/to/lodash.zip/node_modules/lodash), and forwards this path to the real TS resolver (that's parentResolver)

since it doesn't care about resolving it further (and just use the parent resolver), I don't see why it would break the typesVersion selection 🤔

similarly, we don't check "files" / "folders" at all - we just tell TS that instead of resolving lodash/fp it should instead resolve /path/to/lodash.zip/node_modules/lodash/fp. I would expect it to follow the same resolution process based on the type of fp

[skoging]

Running into this same issue when importing from redux-saga/effects.

The file structure of the package is

redux-saga-npm-1.1.3-f4b0ce38ee
└── node_modules
    └── redux-saga
        ├── LICENSE
        ├── README.md
        ├── dist
        ├── effects
        │   └── package.json
        ├── effects.d.ts
        ├── index.d.ts
        └── package.json

The effects folder overrides the effects.d.ts file, but effects/package.json has no types field:

// effects/package.json
{
  "name": "redux-saga/effects",
  "private": true,
  "main": "../dist/redux-saga-effects-npm-proxy.cjs.js",
  "module": "../dist/redux-saga-effects-npm-proxy.esm.js"
}

Adding a types field resolves the issue, but is not needed with tsc: "types": "../effects.d.ts"

---

For anyone looking for a temporary solution:

redux-saga redirects all imports to @redux-saga/core, and importing from @redux-saga/core/effects works without issues.

[leightonsmith]

thanks for the detailed writeup @skoging. I'm not working with typescript anymore, but I hope this helps the ts-pnp folks.

[pauldraper]

ts-pnp simply implements a thin conversion that transforms the bare import

ts-pnp adds a trailing slash. To quote the code,

      // TypeScript checks whether the directory of the candidate is a directory
      // which may cause issues w/ zip loading (since the zip archive is still
      // reported as a file). To workaround this we add a trailing slash, which
      // causes TypeScript to assume the parent is a directory.

ZIP file aren't part of PnP, but some Yarn idiosyncracy that this plugin accommodates, in so doing breaks this case of

lodash/fp
lodash/fp/fp.d.ts