search

antfu-collective / package-manager-detector

Package manager detector
MIT License
36 stars 4 forks source link

feat: add `npm_config_user_agent` support #20

Closed userquin closed 1 month ago

userquin commented 1 month ago

Description

Linked Issues

resolves #19

Additional context

benmccann commented 1 month ago

I think this should be a new method rather than updating the existing method (https://github.com/antfu-collective/package-manager-detector/issues/19#issuecomment-2411563002)

Updating the existing method would be a breaking change as I think there are places I'm using the existing method to detect which pm is used in the project rather than the one currently running.

E.g. see https://github.com/zkochan/packages/tree/main/preferred-pm vs https://github.com/zkochan/packages/tree/main/which-pm-runs

userquin commented 1 month ago

Why breaking? The default option value is undefined and will not check for the env entry

benmccann commented 1 month ago

Thanks for the update! Yeah, not a breaking change

Though I do think separate methods would be better for tree shaking. Right now we basically have if (opt) { method1() } else { method2() }. Why not just let the user call the method they need directly to avoid including code for the method they don't need?

MichaelDeBoey commented 1 month ago

Now that I created https://github.com/antfu-collective/package-manager-detector/issues/21, I'm not sure if using npm_config_user_agent would (always) be the correct thing to look at though 🤔

benmccann commented 1 month ago

Now that I created https://github.com/antfu-collective/package-manager-detector/issues/21, I'm not sure if using npm_config_user_agent would (always) be the correct thing to look at though 🤔

We can add support for Deno in this method when implementing https://github.com/antfu-collective/package-manager-detector/issues/21. I'll comment there with some more details. I don't think it should block this PR though