Skip to content
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

feat(yarn-pnp): adding support for turbo run w/ yarn pnp #797

Merged
merged 3 commits into from
Mar 8, 2022

Conversation

thebanjomatic
Copy link
Contributor

@thebanjomatic thebanjomatic commented Mar 1, 2022

Regarding #693 (comment) I believe this at least partially addresses Issue: #693, but the scope of the issue is unclear.

More specifically this removes the general check for yarn pnp and moves it into the prune command instead as this is the part that likely has remaining issues with pnp since nothing else that I'm aware of needs to know about node_modules.

This PR builds off of #795, as otherwise you can't even run turbo to be able to hit the previous nodeLinker checks.

Let me know if you would like me to change things around so that it is more explicitly enabling for the run task as opposed to enabling for all but prune.

@vercel
Copy link

vercel bot commented Mar 1, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/vercel/turbo-site/8gFsXo25kYxmztMtL4ke9qB7xUJG
✅ Preview: https://turbo-site-git-fork-thebanjomatic-feat-yarn-pnpenable-run.vercel.sh

@thebanjomatic
Copy link
Contributor Author

I got a CI test failure on windows-latest, re-running seems to have 'fixed' it. The test failure happens for me locally as well, but also shows up in main and when checking out the v1.1.4 tag, so I don't think its new behavior introduced by this change.

More specifically this removes the general check for yarn pnp and moves it into the
prune command instead as this is the part that likely has remaining issues with pnp.
@thebanjomatic
Copy link
Contributor Author

@jaredpalmer / @gsoltis is there any additional testing that needs to be done to be able to move this PR forward?

@vercel
Copy link

vercel bot commented Mar 8, 2022

@thebanjomatic is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@thebanjomatic
Copy link
Contributor Author

thebanjomatic commented Mar 8, 2022

@gsoltis Is there a way we can get a canary release from this PR so that other consumers with yarn 2 PnP repos can test and validate this change? I have verified that its working in my own repos at this point, but I'm given there wasn't any actual code change made on my part to enable this functionality, I can understand there may be some uncertainty around this PR.

@kodiakhq kodiakhq bot merged commit a0eebfb into vercel:main Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants