-
-
Notifications
You must be signed in to change notification settings - Fork 181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Yarn 2.x #670
Comments
Not really. I don't know exactly how pnp works and the last time I checked it required executing node to resolve the dependencies since for some reason they aren't available as data. shadow-cljs currently resolves all dependencies on its own without ever executing node. So yeah this is very far from simple. I can see if I can figure it out when I have some time but for now I'd recommend sticking with regular |
I am using Yarn 1.x. I tested the current preview of 2.x to test it and relay feedback. 2.x is not released yet so I was just being nice to alert you of the upcoming changes to be able to support it when that happens. shadow-cljs may not be calling Per your own requirements:
|
I suggest taking a look at Yarn's roadmap (yarnpkg/yarn#6953) as it gives an explanation as to what is changing and how it will be easier used. This is why I said it should be trivial to implement in shadow-cljs. Specifically:
|
Some discussion on cross language Yarn PnP integration here: evanw/esbuild#154 (comment) |
Typescript has been resistant to adopting Yarn PnP, and going over their position can give insight on why would a build tool not be inclined to support PnP right now:
Yarn PnP is interesting and solves some shortcomings present in npm and the general node resolution algorithm, but it is at its core an opt-in approach that requires changes in any tool that relies on their own implementation of the node resolution algorithm. |
I found a solution to this is simply to use the built-in node-modules plugin by placing a
This causes packages to be copied into Therefore I suggest documenting the above as the solution rather than adopting PnP. Thoughts @thheller ? |
@superstructor this is pretty much the same as not using yarn v2 in the first place so probably not what people are after. |
Maybe. But for me, Yarn 2 is more interesting as a maintained alternative to NPM than for dropping All of Yarn 2's arguments for dropping |
Just to add, one of the big reasons for Yarn 2's pnp system can also be disk space usage. The current approach is to have yarn v1 installed globally and yarn v2 installed on a per project basis. Then it generates a .pnp.js file in the project which I think is somehow used to access these modules virtually. |
Sorry for the late reply on this. I'm not sure I understand the backlash you're referencing here. Perhaps I'm misunderstanding some behavior of PnP, but from my understanding it is designed to enable custom behavior interop by specifying it as an API. I'm not sure I see the parallel between what TypeScript and shadow-cljs is trying to accomplish. TypeScript needs to add searching for types along with the regular dependencies. On the other hand, shadow-cljs is just pulling in the JS dependencies, no? One of the comments in the PR is capturing what I was originally suggesting at the start of this thread:
So by that reasoning, couldn't we enable support of Yarn 2.x by using the PnP logic conditionally based on the presence of |
@thheller I have time available now to assist on this. I'll take a deep dive through the existing code base to gain a better understanding of your current process. |
@WhoNeedszZz the "shortest" path to integrate this would be implementing a function like
I'd assume the
There are many examples for this pattern throughout the codebase. I'd be ok for everything to live in shadow-cljs/src/main/shadow/build/resolve.clj Lines 67 to 68 in b06ef3a
Basically there would be a |
given this solution, it could also probably work with |
Yarn is at 4.x nowadays and I don't have a clue if any of this is still relevant. If anyone wants to take this on as described in #670 (comment) please open a new issue. |
Yarn has made some significant changes in Yarn 2.x. It no longer uses node_modules/ and instead uses a virtual filesystem via PnP (https://yarnpkg.com/features/pnp). Due to this change, shadow-cljs is not compatible with it as it looks for node_modules/ to exist, which it no longer does. It seems like it would be fairly simple to look for node_modules/ as well as use .pnp.js to support npm and Yarn 1.x as well as Yarn 2.x.
The text was updated successfully, but these errors were encountered: