fix(create-rezi): harden windows scaffold installs#393
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91bd2454ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (npmExecPath) { | ||
| return { command: nodeExecPath, args: [npmExecPath, "install"] }; |
There was a problem hiding this comment.
Respect --pm npm instead of always trusting npm_execpath
In resolveInstallInvocation, any truthy npm_execpath is executed when packageManager === "npm", but npm_execpath is often set by non-npm launchers (for example pnpm/yarn script contexts). In that case, create-rezi --pm npm can run the wrong installer (or fail) because it invokes node <pnpm-or-yarn-cli> install instead of npm, which breaks the explicit --pm contract and regresses prior behavior that always executed npm install.
Useful? React with 👍 / 👎.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIntroduces a Windows-safe install launcher using Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as create-rezi CLI
participant Resolver as resolveInstallInvocation
participant Spawner as cross-spawn.sync
participant Installer as npm/pnpm (or node exec)
CLI->>Resolver: provide packageManager, env, platform, nodeExecPath
Resolver-->>CLI: return { command, args }
CLI->>Spawner: call sync(command, args, options)
Spawner-->>Installer: spawn process (npm/pnpm or node + npm_execpath)
Installer-->>CLI: install exit/status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Validation
Summary by CodeRabbit
+binding with=(andShift+=on Windows) while preserving+as a command alias, ensuring consistent behavior across platforms.