Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Native support for PnP #35206
Native support for PnP #35206
Conversation
| return typeRoots; | ||
| } | ||
| const nodeModulesAtTypes = combinePaths("node_modules", "@types"); | ||
|
|
||
| function getPnpTypeRoots(currentDirectory: string) { |
arcanis
Nov 19, 2019
Author
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).
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).
| const searchResult = loadModuleFromNearestNodeModulesDirectory(Extensions.DtsOnly, typeReferenceDirectiveName, initialLocationForSecondaryLookup, moduleResolutionState, /*cache*/ undefined, /*redirectedReference*/ undefined); | ||
| const searchResult = isPnpAvailable() | ||
| ? tryLoadModuleUsingPnpResolution(Extensions.DtsOnly, typeReferenceDirectiveName, initialLocationForSecondaryLookup, moduleResolutionState) | ||
| : loadModuleFromNearestNodeModulesDirectory(Extensions.DtsOnly, typeReferenceDirectiveName, initialLocationForSecondaryLookup, moduleResolutionState, /*cache*/ undefined, /*redirectedReference*/ undefined); |
arcanis
Nov 19, 2019
Author
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).
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).
| } | ||
|
|
||
| function getPnpApi() { | ||
| return require("pnpapi"); |
arcanis
Nov 19, 2019
Author
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).
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
Nov 21, 2019
Member
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?
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
Nov 21, 2019
Author
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.
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
Nov 21, 2019
•
Member
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.
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
Nov 22, 2019
Author
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.
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
Nov 26, 2019
Author
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.
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
Nov 26, 2019
Member
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.
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
Nov 26, 2019
+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.
+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
Nov 26, 2019
Author
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 🙂
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
Jan 26, 2020
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?
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?
|
Hopefully we can get support from the TS core team for this one |
|
@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 All that to say that I'm still interested to get this PR merged if there's any interest on your side |
|
In fact we didn't have to wait long since the patch doesn't apply cleanly on TS 3.5 ... |
|
And TS 3.9 seems to be incompatible as well, which will require another copy of the patch. |
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. |
|
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'. |
@weswigham any progress on those requests? |
|
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. |
Is that progress be merging this 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? |
|
Any news about that? |
|
@macabeus yes i have some news on that typescript needs a resolver plugin interface something that it can call to integrate that there exists already some code that goes into that direction you can expect such integration in the next 5 years i think. If some one puts priority on that it is do able in maybe 1-2 month with 2 people but at present no one did something in that direction. I also want such features but i am a single person i would need to focus on that. So it has low priority from my side. but if you get me 3 JS coders that want to do it i can lead them into the right direction and do reviews and that. |
|
@frank-dspeed I'm sorry but I think your project is quite different from what I'm aiming for in this PR. The goal here is to add support for the PnP resolution in TS itself, so anything tied to a specific bundler isn't an option. @macabeus No news regarding this PR itself. I still maintain it, and it's still automatically applied when installing TS with Yarn 2, but we haven't heard anything from the TS team for some time. I still hope it'll be merged someday, but I have no control over that. |
|
What happened about typescript's very own resolver API? |
|
@SagnikPradhan @arcanis there is a new resolve api inside typescript you can use it already to switch between typescript and node resolve via tsconfig this needs to get expanded. |
|
Just to underscore the importance of PnP support by TypeScript. pnpm will also have an option to install dependencies using PnP, so soon two Node.js package managers will support PnP. |
|
@frank-dspeed Again, this PR is purely to discuss about this PR, not alternative strategies (unless the TS team is the one suggesting them, for obvious reasons). As you can guess a significant amount of work has already been spent evaluating the different options, and there's a reason why it ended up being implemented this way. Also note that many people are subscribed to this issue - please let's keep the discussion focused. |
|
@arcanis we love this patch and Yarn berry, we have an issue when trying to use a custom |
|
@weswigham Forgive me for testing out the water again, but pnpm has shipped with PnP in 5.9. Would your team be open to consider that perhaps PnP would make for an acceptable resolver API to support? I'm aware it may not satisfy all your checkboxes, but it's used in production in many projects by now, and despite my best efforts I'm afraid TS support will always be imperfect until your team collaborates directly with us (which we're super open to do, on our side). |
|
We're still not willing to call out to arbitrary JS during our execution without a proper plugin model ready, and the "pnp API" is still arbitrary JS code and not a manifest we can directly consume. |
Note that the PnP implementation shipped by both Yarn and pnpm supports generating an additional static JSON file ( These data structures can be hydrated into a runtime API by either implementing your own logic, or adding a dependency on the |
@arcanis I have always been curious why Plug'n'Play is a plugin: Most programming languages besides JavaScript have one standard way of resolving modules. For example, given a Java binary or some Java source code, an analysis tool can easily determine module mappings -- without executing arbitrary scripts that might get stuck in an infinite loop, or leak memory, or maliciously mine bitcoins or whatever. NPM/Node.js actually demonstrated that many different JavaScript scenarios can all be addressed by a single universal package system and resolver algorithm. Unfortunately the The flexibility of Plug'n'Play seems to be a disadvantage for adoption: Caches can't be sure what inputs might invalidate the cache. Package authors can't be sure which implementations they should test with. And here the TypeScript maintainers are hesitant about loading arbitrary code into their engine. All those problems would disappear if NPM resolution used a standardized algorithm and some standardized config files -- data instead of code. Actually solving the problem instead of outsourcing the solution to plugin code. I'm very new to Plug'n'Play though, so sorry if these questions are misguided. :-) |
|
@octogonz Java is a bad example in this case. Try googling "classpath issue" or "jar hell". Not really inspiring. I agree with many of your other points though, I'd like to hear more motivation for having an executable format for the pnp file. |
Because as always, the question isn't to have a great design but rather to gather support around it. When you consider that even now the only package manager in Node is controlled by a for-profit and that it takes crazy significant efforts to improve even that, I can't imagine the amount of work that a proposal such as Plug'n'Play would require - and how much of my family time it would consume. So of course in an ideal world PnP should be implemented in Node, and I'd be open to help with that (or even do most of the work), but it likely won't happen until the Node TSC comes to us with a firm Intent to Implement so that I know it won't be all for nothing. In the meantime, the best I can do is to document the standard and API the best I can and work with third-party project to make "PnP as a plugin" a reality. I think it worked very well so far, even though there's still this kind of PR every once in a while.
This point in particular got discussed a few times already (for example here), but I think the TS team demonstrated since then why the executable format is just fine: the data format actually got implemented in May 2019 - a year and a half ago - but it didn't do anything to bring PnP closer from TS. Given that it was literally its only purpose, it shows that we should optimize the format for the convenience of our team rather than for theoretical use cases. |
|
@arcanis well, you did not make the data format the default, just opt-in. Until its the default, how could it change anything for TS? TS would still need to execute the pnp js for projects that have not opted for a static data pnp file. |
|
@0rvar users who want native TS+PnP support could just opt-in to the data format. The default can be changed later. |
|
@andreialecu Then I guess the PR should change to only work with the data pnp file. |
|
If the TS team would commit with a firm Intent to Implement, I'm sure a PR that uses the data format will easily come along. But like @arcanis said:
|
|
@arcanis What if you separated your work into two parts, maybe even with different names:
If the spec stabilizes and achieves widespread adoption, then one day we won't need Plug'n'Play any more, because Node.js will implement SmartRequire natively. This would achieve a couple things:
The API seems like a third thing. It's not the Plug'n'Play hack. But it's also not something TypeScript would need to implement in order to resolve a module. It seems more like a handy but optional utility for querying the |
|
I appreciate suggestions, but this thread is already long and many people are subscribed - it's not the right place to discuss strategy Can you repost that on the yarnpkg/berry repository, so that we can keep this PR focused on feedback and updates about this specific implementation? I wouldn't want it to be closed for being off topic |
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
|
Bot, the reference to #28289 (still open) is in the very first post: #35206 (comment) |
arcanis
force-pushed the
arcanis:mael/pnp
branch
2 times, most recently
from
a24be51
to
98889d3
masterbranchgulp runtestslocallyBacklogmilestone (required):Although not in the Backlog milestone, this PR is in reference to #28289
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: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😊