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

Completed npm commands (npm:install, npm:update, npm:run) #1202

Merged
merged 30 commits into from
Oct 25, 2024

Conversation

jaxwilko
Copy link
Member

@jaxwilko jaxwilko commented Sep 12, 2024

This PR removes [vite|mix]:installs ability to install an npm package into the root package.json (a hold over from the previous mix system). In it's place it adds the commands npm:install <compileablePackage?> <npmPackage> which allows the user to install a packge into a workspace.

I.e. if I want to add leftpad to my plugin JaxWilko.Example I can run ./artisan npm:install jaxwilko.example leftpad which will then add the dependency to plugins/jaxwilko/example/package.json then run the install command as the root project owner (root package). This is powered by npm and we just need to run the right command in the workspace package.

This PR also reworks npm:update and npm:run so they share a parent with the npm execution logic and simplifies their internal logic.

NPM Run

npm:run still works as before. ./artisan npm:run jaxwilko.example test will execute the test script within the jaxwilko.example package.

NPM Update

npm:update now supports a package to update, as well as now taking a assetPackage to specify which workspace to run the command within.

  • ./artisan npm:update jaxwilko.example tailwindcss will update tailwindcss within jaxwilko.example
  • ./artisan npm:update tailwindcss will update tailwindcss globally (within the root package, workspaces can still install their own independent version).

NPM Install

  • ./artisan npm:install jaxwilko.example tailwindcss will install tailwindcss within jaxwilko.example
  • ./artisan npm:install tailwindcss will install tailwindcss in the root package.json
  • ./artisan npm:install tailwindcss --dev will install tailwindcss within devDependencies rather than dependencies

TODO

  • New tests
    • npm:install
    • npm:update
    • npm:run
  • Fix detection of ignored packages in asset:install
  • Fix detection of missing configuration and offer to run config creation command in asset:install
  • Restict asset:install to only install requested package and not all unless no package is specified

@josephcrowell
Copy link
Contributor

Yes that's the expected behavior.

@josephcrowell
Copy link
Contributor

josephcrowell commented Sep 15, 2024

In the mean time, until this is merged, what would be the correct npm command to do the equivalent npm update on a plugin or theme?

I was working on a puppeteer based html to pdf plugin specific to winter and capable of using Firefox that I would like to continue development on and a Daisy UI based theme.

@bennothommo
Copy link
Member

@josephcrowell yes, you can use npm directly if you wish. I would be using it on the project root as opposed to within the plugin or theme itself, in order to completely replicate what the artisan command does.

@jaxwilko jaxwilko requested a review from LukeTowers September 24, 2024 15:20
@LukeTowers
Copy link
Member

@jaxwilko are there any changes that could be made to the docs?

@LukeTowers LukeTowers added enhancement PRs that implement a new feature or substantial change needs response Issues/PRs where a maintainer is awaiting a response from the submitter labels Sep 24, 2024
@jaxwilko
Copy link
Member Author

@LukeTowers probably yes, at least some work on clarification of the expected / actual behaviour, i'll open a PR tomorrow and also add some more comments to the code in this PR to make it more clear what's going on :)

@jaxwilko
Copy link
Member Author

jaxwilko commented Oct 3, 2024

Docs update PR: wintercms/docs#213

@LukeTowers
Copy link
Member

@jaxwilko what's left?

@jaxwilko
Copy link
Member Author

jaxwilko commented Oct 9, 2024

@LukeTowers just reworking the exceptions

@bennothommo bennothommo removed the needs response Issues/PRs where a maintainer is awaiting a response from the submitter label Oct 11, 2024
@jaxwilko
Copy link
Member Author

@LukeTowers custom exceptions have been removed and replaced with SystemException

@LukeTowers LukeTowers changed the title [WIP] Added inital work on npm commands Completed npm commands (npm:install, npm:update, npm:run) Oct 25, 2024
@LukeTowers LukeTowers merged commit 9e8d01c into develop Oct 25, 2024
13 checks passed
@LukeTowers LukeTowers added this to the 1.2.7 milestone Oct 25, 2024
@LukeTowers LukeTowers deleted the wip/npm-commands branch October 25, 2024 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement PRs that implement a new feature or substantial change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants