Add NodeJS website using mise supporting node versions per site #974
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request introduces Node.js website support using mise (a runtime version manager) to enable per-site Node.js version management. It replaces the legacy Node.js implementation that relied on system-wide installations with a more flexible approach that allows different sites to use different Node.js versions.
Key changes:
- Implemented mise integration for runtime version management with installation and verification scripts
- Created a new
MiseNodeJSsite type that supports configurable Node.js versions (16, 18, 20, 22) per site - Renamed the existing NodeJS site type to
LegacyNodeJSto maintain backward compatibility
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/TestCase.php | Added NodeJS service setup to test infrastructure |
| tests/Feature/SitesTest.php | Added test cases for both MiseNodeJS and LegacyNodeJS site types with various configurations |
| resources/views/ssh/os/install-dependencies.blade.php | Replaced Node.js installation with mise installation during server setup |
| resources/views/ssh/mise/install-runtime.blade.php | Created script to install specific mise runtime versions |
| resources/views/ssh/mise/ensure-installed.blade.php | Created script to verify/install mise on the server |
| app/SiteTypes/MiseSiteType.php | Created abstract base class for mise-based site types with runtime management |
| app/SiteTypes/MiseNodeJS.php | Implemented new Node.js site type using mise for version management |
| app/SiteTypes/LegacyNodeJS.php | Renamed from NodeJS to LegacyNodeJS to support existing implementations |
| app/SSH/Mise/Mise.php | Created mise SSH interaction class for runtime installation and verification |
| app/Providers/SiteTypeServiceProvider.php | Updated site type registration to include both MiseNodeJS and LegacyNodeJS with appropriate forms |
| app/Providers/ServiceTypeServiceProvider.php | Updated label to indicate legacy Node.js service |
| .easytree.json | Added project configuration file (may be unintentional) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
app/SiteTypes/MiseSiteType.php
Outdated
| { | ||
| $version = $this->runtimeVersion(); | ||
|
|
||
| return 'mise exec '.$this->runtime().'@'.$version.' --'; |
There was a problem hiding this comment.
The runtimePrefix() method may construct an invalid command when runtimeVersion() returns null. This would result in "mise exec node@ --" which is malformed. Consider adding a null check or ensuring the version is always provided when this method is called.
| return 'mise exec '.$this->runtime().'@'.$version.' --'; | |
| $command = 'mise exec '.$this->runtime(); | |
| if ($version) { | |
| $command .= '@'.$version; | |
| } | |
| return $command.' --'; |
| ->select() | ||
| ->label('Node.js Version') | ||
| ->options(MiseNodeJS::NODE_VERSIONS) | ||
| ->default('22'), |
There was a problem hiding this comment.
The default version in the form is '22' but the default in the data() method is '20'. This inconsistency could lead to unexpected behavior. Consider aligning both defaults to the same version.
| ->default('22'), | |
| ->default('20'), |
| "scripts": [ | ||
| "composer install", | ||
| "cp $PROJECT_PATH/.env .env" | ||
| ] |
There was a problem hiding this comment.
This file appears to be unrelated to the PR's purpose of adding NodeJS website support using mise. The .easytree.json configuration seems to be a project setup file that was accidentally included in this pull request. Consider removing it if it's not intentionally part of this change.
| "scripts": [ | |
| "composer install", | |
| "cp $PROJECT_PATH/.env .env" | |
| ] |
| private function legacyNodeJS(): void | ||
| { | ||
| RegisterSiteType::make(LegacyNodeJS::id()) | ||
| ->label('NodeJS with NPM (Legacy)') |
There was a problem hiding this comment.
Inconsistent casing in labels: "Node.js" is used for the new MiseNodeJS type (line 140), but "NodeJS" is used for the LegacyNodeJS type (line 172). Consider using consistent casing such as "Node.js" for both to maintain naming consistency across the UI.
| ->label('NodeJS with NPM (Legacy)') | |
| ->label('Node.js with NPM (Legacy)') |
closes #971