Native support for PnP

[arcanis]

• Code is up-to-date with the master branch
• You've successfully run gulp runtests locally
• There is an associated issue in the Backlog milestone (required):
  Although not in the Backlog milestone, this PR is in reference to Add new moduleResolution option: yarn-pnp #28289
• There are new or updated unit tests validating the change:
  Not yet because it's just a PoC for now and I wanted to hear the opinion of the TS team first

What is PnP? (you can skip if you already know)

It's an alternate install strategy where instead of installing the files within the node_modules, we instead tell Node to directly reference them from a flat location on the disk. The resulting installs are more space-efficient and faster, but aren't compatible with programs that reimplement the node_modules traversal algorithm.

Because it involves a different resolution algorithm than the default Node one it can be seen as a relatively high-risk endeavour. Conscious of that, we've made significant progress during this past year to remove obstacles one by one. We welcomed new community members, helped many project to fix their package definitions, improved our implementation to provide more detailed errors, and thanks to everyone's collaboration the remaining blockers are only a handful. My point here is: PnP isn't just a phase - just like Yarn itself didn't end up just a phase.

PnP and TypeScript

We've discussed a bit with TS not so long ago, and from my understanding some tools might appear in the future to make it easier to implement this kind of logic outside of the core. Given that Yarn will likely be the prime consumer, I figured it could be interesting to try and see how PnP support could be implemented natively - to get a better idea of the tradeoffs involved, etc. A basic analysis yielded yarnpkg/berry#589, and I decided to spent a couple of hours trying to actually make it work.

This diff makes TS able to query the PnP runtime to obtain the location of the file dependencies. Because it doesn't require files that haven't been executed already (cf isPnpAvailable) I believe it should also address the security concerns initially raised by the TS team. I tested it with the Berry repository itself and it typechecked everything correctly:

git clone git@github.com:yarnpkg/berry berry
git clone git@github.com:arcanis/typescript arca-ts
(cd arca-ts && git checkout mael/pnp && yarn && yarn gulp)
(cd berry && yarn node ../arca-ts/built/local/tsc.js --noEmit -p .)

The one missing part is watch support; adding or removing packages requires restarting the server. This part seemed fairly different from the module resolution, so I thought I should first get your opinion on this PR as it is before going any further 😊

[arcanis]

Under PnP environments, the packages are installed like this:

/project/.yarn/cache/@types-foo-1.0.0.zip/node_modules/@types/foo/package.json
/project/.yarn/cache/@types-bar-1.0.0.zip/node_modules/@types/bar/package.json
/project/.yarn/cache/@types-baz-1.0.0.zip/node_modules/@types/baz/package.json

This function obtains the location of all @types dependencies, then retrieve their parent directories to build up to a list of N type roots which each contain a single package (instead of a single type roots that contains N packages as you'd get with typical installs).

[arcanis]

This only affects projects using PnP projects (because of the isPnpAvailable check).

The reason I went with a branch rather than a fallback is because I'm worried of potential resolution corruptions that would happen if for example someone installs a project with Yarn 1 (which would generate a node_modules) then Yarn 2. In this situation, a fallback could get passing results that would fail on pure Yarn 2 systems (because of hoisting).

[arcanis]

The pnpapi module is a builtin module that's only available within the PnP runtime. It always points to the PnP runtime currently active (so by extension, it doesn't execute anything from the disk - similar to if you were calling require('module'), for example).

[weswigham]

How would a PnP runtime be active? Neither the tsc executable we ship nor vscode's language service would do such a thing; so this entrypoint is for.... what kind of usage?

[arcanis]

The runtime is active when running TypeScript through yarn. Running yarn run tsc setups NODE_OPTIONS to first load the loader before pursuing the execution.

For VSCode, we use typescript.tssdk to point to a tsserver that firsts setups the hook and then require the real tsserver. In both cases, those paths require explicit user interaction.

[weswigham]

I'm going to go out on a limb and say that this kind of ad-hoc cooperative plugin model (least of all, one that explicitly ties the executed plugin to specifically pnp-related runtime code, and not "arbitrary runtime-provided host behavior" like the ChakraHost hook provides) isn't something we'd be interested in compared with the very explicit model @rbuckton has been working on, but we'll see I guess.

[arcanis]

One thing I'd like to mention is that the PnP API is specified in a way that leaves open various implementations. I'm fairly sure the node_modules resolution could be described by the exact same API, for example.

So in a sense (and that's definitely not an accident; that's part of the reasons why I made it an API rather than say a JSON file), PnP can be seen as a plugin interface in itself, separate from the way Yarn implements it.

[arcanis]

I understand. At the same time this API has now been there for more than a year now and I think we've proved time and again during this period that we're committed to keep developing it - putting special care in our documentation, providing context to our neighbours, providing direct contributions, etc.

PnP will stay - even once the loaders are ready (and I don't expect them to be for a very long time, if ever when it comes to commonjs packages) it will only change how the runtime is injected into the environment. PnP itself will keep the same API, with the pnpapi builtin etc. Adding support for it now doesn't preclude you from supporting more complex schemes down the road, and only unlocks use cases that people care strongly about at what appears a minimal cost - especially considering that me or one of my fellow contributors will always be available to help maintain it.

And of course, it would also simply be a nice and appreciated way to support our team. We believe the PnP resolution will play a large part in Yarn in the future, and TS having builtin support would help us a lot by removing the single main blocker that we can't fix by ourselves. Succeeding alone¹ is hard, hence why we ask for your help.

---

¹ I'm overdramatising a bit here - we've worked with many projects already, and for example Webpack 5 (and Gatsby / Next / CRA / Jest / ...) will support PnP out of the box. Still, TS is now central enough to many workflows - including ours - that its lack of support adds a significant weight on us.

[weswigham]

I'm not saying we don't want to enable this kind of thing, I'm just saying we have little interest in one-off PnP-specific things, when a more complete approach would render it redundant and allow for more use cases to be filled, and that this model, specifically, follows a pattern that we don't consider particularly usable (we should know, it is similar in setup to our existing "language service plugin" architecture that doesn't layer plugins easily, isn't easy to develop for, isn't particularly easy to use, isn't (well) documented, and and was rushed out to support a high pressure request from angular). We've had requests for custom resolver behavior for approximately forever, and would like to do right by those, and not repeat the mistakes we've already made in a different context.

[alexpomsft]

+1 to solving the bigger custom resolver problem in a good way, that supports multiple resolvers playing nicely with each other. For example, we would like to be able to build a resolver to solve React Native file extension resolution and chain it together with a yarn pnp resolver.

[arcanis]

If there's a way to do that, now or in the next months, I'm all for it 👍

My anxiety is mostly that as you mentioned, there have been similar requests for a long time. Plugin systems are notoriously hard to get right, and you understandably want to be sure it won't bite you in the future. That's fine - but meanwhile, we're still left in a state where we have to rely on very creative solutions (a virtual filesystem!) where a simple integration would appear to work fine - within the constraint of our runtime, admittedly.

Anyway, I understand what you're saying and your roof, your rules. I'll be eagerly waiting for progress on #16607 then 🙂

[Ore4444]

wait, isn't pnpapi just an implementation detail of the compiler? The logic could all be conditional to presence of 'pnpapi' module. When npm would settle on its approach, supported could be added without breaking changes, no?

[sgurenkov]

Hopefully we can get support from the TS core team for this one
Lacking native support in TS is the only thing that stops us from migrating to PnP

[arcanis]

@weswigham fyi the patch-on-install approach we've been following has worked fine so far (kudos to your team btw - even when we see bugs, they aren't that hard to locate and fix).

That being said, the architecture isn't as clean as it could be since we have to prioritize small diffs over sound design (hence why isPnpAvailable is duplicated in three files, for example).

All that to say that I'm still interested to get this PR merged if there's any interest on your side 🙂

[arcanis]

In fact we didn't have to wait long since the patch doesn't apply cleanly on TS 3.5 ...

[arcanis]

And TS 3.9 seems to be incompatible as well, which will require another copy of the patch.

[pauldraper]

I'm just saying we have little interest in one-off PnP-specific things, when a more complete approach would render it redundant and allow for more use cases to be filled....We've had requests for custom resolver behavior for approximately forever, and would like to do right by those

+1 to solving the bigger custom resolver problem in a good way, that supports multiple resolvers playing nicely with each other.

But that is PnP: a standardized resolver API, developed by one of the major JavaScript dependency managers and already supported by dozens of the industries most common tools (tsc being a conspicuous exception).

It's being delivered here on a silver platter by @arcanis, but tsc wants(?) to introduce its own standardized resolver API to the mix that is adopted(?) by other tools like Node.js and webpack? And it will have some capability that pnpapi somehow lacks?

How many years will it take for tsc to implement its own "more complete approach," given that it has already been requested "approximately forever."

The NIH is reaching extreme levels.

[typescript-bot]

It looks like you've sent a pull request to update our 'lib' files. These files aren't meant to be edited by hand, as they consist of last-known good states of the compiler and are generated from 'src'. Unless this is necessary, consider closing the pull request and sending a separate PR to update 'src'.

[pauldraper]

We've had requests for custom resolver behavior for approximately forever, and would like to do right by those

@weswigham any progress on those requests?

[weswigham]

We've redone our node factory API for 4.0, which was one of the blockers. We'll possibly see more progress once 4.0 ships.

[pauldraper]

We'll possibly see more progress

Is that progress be merging this "moduleResolution": "pnp" PR? Or something else?

Right now, people are patching typescript in postinstall (yarn does this automatically) to get this working; it works, though it's pretty ugly.

Can it possibly be an experimental feature, like --experimentalDecorators was?

[Kurt-von-Laven]

• As you mention there's also the matter of handling files from within their zip packages. Perhaps that could be solved by allowing a plugin to modify the compilerHost object, so they could update the readFile functions as demonstrated in the API documentation?

This to me is the most problematic part; my hope was that the fact that you have to effectively run all PnP code with the PnP loader via yarn would mean that the FS is already patched and so a plugin would not need to be trying to touch the filesystem at all (so, would be pure resolution, as in this PR). But, I suppose theoretically, the FS patch could execute "late" as part of the plugin load to still hack Node.

But, it's also true that LS plugins can mess with the LS host filesystem, so, it's probably not weird to think about the same in the compiler.

@jakebailey how do you feel about the pros and cons of a compiler plugin touching the filesystem at this point?

[vantaboard]

Fingers crossed this gets through the finish line soon.

[adrian-gierakowski]

this PR is the second most up-voted in this repo

I haven't got time read the entire thread. Would someone from TS team be so kind to post update on their current view on this (as a team, not as an individual). Then the author of the PR could link to this post from the main PR comment to make it easier for passers by to understand where things are at. Thanks!