search

oclif / test

test helpers for oclif components
MIT License
17 stars 11 forks source link

feat: support for Yarn4 PnP #495

Closed ClayChipps closed 7 months ago

ClayChipps commented 7 months ago

This enables this project to be compatible with Yarn4 PnP. Specifically:

I am wanting to work towards the challenge that was poised in oclif/oclif#1222. The first issue that I ran into when attempting to convert @oclif/core to support Yarn4 PnP was that a host of unit tests were failing due to the loadConfig function defined in this project.

mdonnalley commented 7 months ago

@ClayChipps thanks for the PR 🏆

I have a few thoughts/questions:

ClayChipps commented 7 months ago

Thanks for the quick feedback @mdonnalley! I wasn't sure what the long term plan was for packager managers for oclif/sf, so good to know that npm is the target that the projects are moving towards. Is npm the planned package manager of the future for sf too?

Given that, I will make some changes to this PR. The main thing that we have to do to be PnP compatible is to properly manage the ghost dependencies.

mdonnalley commented 7 months ago

I wasn't sure what the long term plan was for packager managers for oclif/sf

Yeah sorry about that. One of the things I want to do this year for oclif is to publish a roadmap that everyone can see and comment on so that people don't end up making PRs that don't align with the roadmap.

Is npm the planned package manager of the future for sf too?

Maybe? We haven't really made any plans for it. For context, plugin-plugins currently uses yarn v1 to manage installed plugins but we're moving that to npm (PR, sf announcement) since we can't move to the latest versions of yarn because they're not published as npm packages anymore. Plugins can continue to use whatever package manager they want after that PR is merged. The only advantage of using npm is that there's more congruency between development and production since npm, yarn, and pnpm all use different dependency resolution algorithms.

Given that, I will make some changes to this PR. The main thing that we have to do to be PnP compatible is to properly manage the ghost dependencies.

Sounds good - really appreciate you taking this on!

ClayChipps commented 7 months ago

since we can't move to the latest versions of yarn because they're not published as npm packages anymore.

Just curious more than anything, but are you all specifically trying to stay away from corepack for now as its experimental?

mdonnalley commented 7 months ago

Just curious more than anything, but are you all specifically trying to stay away from corepack for now as its experimental?

I looked into corepack. Besides the fact that it's still experimental, I was concerned about needing to execute corepack enable on customers' machines - I'm wary of their being some weird side effects there.

But it's more accurate to say that I want to avoid yarn. I don't like that users can globally configure the version of yarn that they want to use. So even if plugin-plugins thinks it's using v4.0 it might actually be using v2 or v3. That opens up the possibility for plugin installs to break based on users' configuration - which I want to avoid. So I'd greatly prefer being able to point directly to the npm executable that available in node_modules.

It's possible that there's something I missing here and corepack has a solution for but npm has the majority market share when it comes to package managers so it feels like a safer, more stable long term solution.

Happy to continue the conversation but https://github.com/oclif/plugin-plugins/pull/776 might be a better place for it in case others want to chime in

ClayChipps commented 7 months ago

@mdonnalley So this is rather...embarrassing...but it appears things are working fine as is with PnP. Closing this unless I find otherwise.