search

koltyakov / generator-sppp

🐾 SP Pull-n-Push - Yeoman generator for SharePoint client-side applications
MIT License
64 stars 12 forks source link

added support for PNPM package manager (flag --pnpm) #19

Closed olegrumiancev closed 6 years ago

olegrumiancev commented 6 years ago

Adding support for PNPM package manager, invokable by yo sppp --pnpm boolean flag

Current PNPM internal (git resolver) issue https://github.com/pnpm/pnpm/issues/1127 prevents installation from GitHub repos pointing to commits with short hashes.

In our case issue is with "sp-build-tasks" dependency

"source-map-explorer": "git+https://github.com/danvk/source-map-explorer.git#b74f71876d97de90b4e69a8b8b2e42388e2393be",

olegrumiancev commented 6 years ago

...otherwise the code would work 🤣

koltyakov commented 6 years ago

Cool! Thanks, Oleg! I'll take a look tomorrow. Maybe source-map-explorer already merged the fix because of which I used git reference and I can use a regular one.

olegrumiancev commented 6 years ago

Happy to help, man :) He did! On Aug 13th, v 1.6.0 https://github.com/danvk/source-map-explorer/issues/76

koltyakov commented 6 years ago

Looks good. Once again, thanks! My plan is: update sp-build-tasks and merge the change over the weekend.

A question: how about not providing a flag for pnpm as a boolean, but ask for a package manager name to use? E.g. I check yarn presence currently and use it as a preferred package manager, but caught myself on the fact I started using npm more often even having yarn installed. As latest npm is pretty equal to what yarn offers but provides "the power of defaults".

olegrumiancev commented 6 years ago

I thought about it, too, just as they have done in generator-sharepoint. Do you want to scrap this PR or merge it and then re-do the code a bit? Or, if you want I can modify my code without closing PR and the changes should persist in PR as well?

koltyakov commented 6 years ago

Will just probably merge then apply the minor changes if we'll decide to change the parameter. I consider that --package-manager pnpm or --package-manager yarn can be more universal. What if tomorrow znpm will be the thing. =)

olegrumiancev commented 6 years ago

Cool, ok! I was also wondering why SPFx does it as a flag, and not as a question, and why won't we?

koltyakov commented 6 years ago

Merged, tweaked, tested & published. Works really great, thank you, Oleg! Changed the option as discussed to --package-manager (or --pm). In readme.