Conversation
Add support for using npm packages as templates when creating new Rspeedy projects. This feature allows users to specify custom templates from npm registry with optional version control. Key features: - Support multiple npm template formats (npm:, @scope/package, package-name) - Add --template-version flag for version specification - Implement smart caching mechanism (.temp-templates/) - Support flexible template structures (template/, templates/app/, root) - Isolated installation to prevent workspace conflicts Example usage: npm create rspeedy@latest my-project -- --template my-template npm create rspeedy@latest my-project -- --template @scope/template npm create rspeedy@latest my-project -- --template my-template --template-version 1.2.3 Inspired by sparkling's npm template implementation. Co-Authored-By: [Migrax] <lynx@bytedance.com>
|
📝 WalkthroughWalkthroughAdded npm package template support to create-rspeedy, enabling users to specify custom templates via CLI flags. Introduced a new template-manager module to handle npm package resolution, caching, and template directory discovery, alongside documentation and modified entry point logic for template routing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Adds support to create-rspeedy for using npm packages as project templates, including a version flag and local caching, and documents the new workflow.
Changes:
- Added an npm template resolver/cacher (
template-manager.ts) that installs a template package and copies out a supported template directory layout. - Extended the CLI entry (
index.ts) to parse--template-versionand route npm templates through a temporary localtemplate-*directory forcreate-rstack. - Updated
README.mdwith usage examples, supported structures, and caching behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| packages/rspeedy/create-rspeedy/src/template-manager.ts | Implements npm template detection, install, template-directory selection, and caching. |
| packages/rspeedy/create-rspeedy/src/index.ts | Adds --template-version parsing and an npm-template execution path that feeds create-rstack a temporary local template. |
| packages/rspeedy/create-rspeedy/README.md | Documents npm template usage and expected package structure/caching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const templateDir = path.join(process.cwd(), '.temp-templates', cacheKey) | ||
| const installRoot = path.dirname(templateDir) | ||
| const packagePath = path.join(installRoot, 'node_modules', normalizedName) | ||
|
|
||
| // Check if we should reuse cache | ||
| const forceLatest = options?.forceLatest ?? versionSpecifier === 'latest' | ||
| const shouldReuseCache = !forceLatest && fs.existsSync(templateDir) | ||
|
|
||
| if (shouldReuseCache) { | ||
| return templateDir | ||
| } | ||
|
|
||
| // Create isolated package.json to prevent workspace conflicts | ||
| const anchorPkgJson = path.join(installRoot, 'package.json') | ||
| if (!fs.existsSync(anchorPkgJson)) { | ||
| const minimal = { name: 'rspeedy-template-cache', private: true } | ||
| fs.writeFileSync( | ||
| anchorPkgJson, | ||
| `${JSON.stringify(minimal, null, 2)}\n`, | ||
| 'utf8', | ||
| ) | ||
| } |
There was a problem hiding this comment.
resolveNpmTemplate writes .temp-templates/package.json and runs npm install with cwd: installRoot, but it never ensures installRoot exists. On a fresh run this will throw ENOENT / fail because .temp-templates/ hasn't been created yet. Create the directory (e.g., fs.mkdirSync(installRoot, { recursive: true })) before writing the anchor package.json and before invoking npm.
| try { | ||
| execSync( | ||
| `npm install ${normalizedName}@${versionSpecifier} --no-save --package-lock=false --no-audit --no-fund --silent`, | ||
| { | ||
| cwd: installRoot, | ||
| stdio: 'pipe', | ||
| }, | ||
| ) |
There was a problem hiding this comment.
execSync is invoked with a shell command that interpolates normalizedName/versionSpecifier directly. Since --template comes from user input, this is vulnerable to shell injection (e.g., package name containing ; / &&). Use spawnSync/execFileSync with an argument array (no shell) and validate the package spec before executing.
| // Check if we should reuse cache | ||
| const forceLatest = options?.forceLatest ?? versionSpecifier === 'latest' | ||
| const shouldReuseCache = !forceLatest && fs.existsSync(templateDir) | ||
|
|
||
| if (shouldReuseCache) { | ||
| return templateDir | ||
| } | ||
|
|
||
| // Create isolated package.json to prevent workspace conflicts | ||
| const anchorPkgJson = path.join(installRoot, 'package.json') | ||
| if (!fs.existsSync(anchorPkgJson)) { | ||
| const minimal = { name: 'rspeedy-template-cache', private: true } | ||
| fs.writeFileSync( | ||
| anchorPkgJson, | ||
| `${JSON.stringify(minimal, null, 2)}\n`, | ||
| 'utf8', | ||
| ) | ||
| } | ||
|
|
||
| // Install the package | ||
| try { | ||
| execSync( | ||
| `npm install ${normalizedName}@${versionSpecifier} --no-save --package-lock=false --no-audit --no-fund --silent`, | ||
| { | ||
| cwd: installRoot, | ||
| stdio: 'pipe', | ||
| }, | ||
| ) | ||
| } catch { | ||
| throw new Error( | ||
| `Failed to install npm template "${normalizedName}@${versionSpecifier}". Please check if the package exists.`, | ||
| ) | ||
| } | ||
|
|
||
| // Find template directory (by priority) | ||
| const possibleTemplatePaths = [ | ||
| path.join(packagePath, 'template'), // Priority: package/template | ||
| path.join(packagePath, 'templates', 'app'), | ||
| path.join(packagePath, 'templates', 'default'), | ||
| packagePath, // Fallback: package root | ||
| ] | ||
|
|
||
| for (const pathCandidate of possibleTemplatePaths) { | ||
| if ( | ||
| fs.existsSync(pathCandidate) && fs.statSync(pathCandidate).isDirectory() | ||
| ) { | ||
| // Copy to cache directory | ||
| fs.mkdirSync(templateDir, { recursive: true }) | ||
| // eslint-disable-next-line n/no-unsupported-features/node-builtins | ||
| fs.cpSync(pathCandidate, templateDir, { recursive: true }) | ||
| return templateDir |
There was a problem hiding this comment.
When forceLatest is true (or the cache doesn't exist), the code mkdirSync(templateDir, { recursive: true }) and then cpSync into it, but it never clears an existing templateDir. Reinstalling latest (or re-resolving the same version after a partial run) can leave stale files mixed with new ones. Remove the existing cache dir (or copy into a fresh temp dir and atomically replace) before copying.
| // Handle npm template specially | ||
| if (typeof argv.template === 'string' && isNpmTemplate(argv.template)) { | ||
| const templateVersion = argv.templateVersion ?? argv['template-version'] | ||
| const templatePath = resolveCustomTemplate( | ||
| argv.template, | ||
| templateVersion, | ||
| ) | ||
|
|
||
| // Create a temporary template directory name | ||
| const templateName = `npm-${Date.now()}` | ||
|
|
||
| // Copy npm template to a temporary local template directory | ||
| const tempTemplateDir = path.join( | ||
| path.resolve(__dirname, '..'), | ||
| `template-${templateName}`, | ||
| ) | ||
|
|
||
| // Copy the resolved template to our template directory | ||
| const fs = await import('node:fs') | ||
| fs.cpSync(templatePath, tempTemplateDir, { recursive: true }) | ||
|
|
||
| // Call create with the temporary template | ||
| await create({ | ||
| root: path.resolve(__dirname, '..'), | ||
| name: 'rspeedy', | ||
| templates: [templateName], | ||
| version: devDependencies, | ||
| getTemplateName: async () => templateName, | ||
| mapESLintTemplate: () => null, | ||
| }) | ||
|
|
||
| // Clean up temporary template directory | ||
| fs.rmSync(tempTemplateDir, { recursive: true, force: true }) | ||
| return | ||
| } |
There was a problem hiding this comment.
The npm-template detection is too broad for create-rspeedy: isNpmTemplate('react') / isNpmTemplate('react-ts') returns true, so passing --template react (or any built-in template name) will enter the npm-install path and try to install/copy the react/react-ts npm package instead of using the bundled template-react-* directories. You likely need to disambiguate by preferring local templates when template-${argv.template} exists under root, and only treating it as npm when prefixed with npm: (or when no local template matches).
| function parseArgv(): ExtendedArgv { | ||
| const argv = process.argv.slice(2) | ||
| const result: ExtendedArgv = {} | ||
|
|
||
| for (const arg of argv) { | ||
| const index = argv.indexOf(arg) | ||
| if (arg === '--template-version' || arg === '--tv') { | ||
| result.templateVersion = argv[index + 1] | ||
| result['template-version'] = result.templateVersion | ||
| } else if (arg === '--template' || arg === '-t') { | ||
| result.template = argv[index + 1] | ||
| } else if (arg === '--dir' || arg === '-d') { | ||
| result.dir = argv[index + 1] | ||
| } else if (!arg.startsWith('-') && !result.dir) { | ||
| result.dir = arg | ||
| } | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| return result |
There was a problem hiding this comment.
parseArgv() iterates for (const arg of argv) but then uses argv.indexOf(arg) to find the value. If an argument appears more than once, indexOf always returns the first occurrence, so later flags may read the wrong value. This also makes the loop O(n^2). Consider iterating by index (for (let i = 0; i < argv.length; i++)) and advancing i when you consume a flag value, and/or supporting --template-version=... forms.
| // Copy the resolved template to our template directory | ||
| const fs = await import('node:fs') | ||
| fs.cpSync(templatePath, tempTemplateDir, { recursive: true }) | ||
|
|
||
| // Call create with the temporary template | ||
| await create({ | ||
| root: path.resolve(__dirname, '..'), | ||
| name: 'rspeedy', | ||
| templates: [templateName], | ||
| version: devDependencies, | ||
| getTemplateName: async () => templateName, | ||
| mapESLintTemplate: () => null, | ||
| }) | ||
|
|
||
| // Clean up temporary template directory | ||
| fs.rmSync(tempTemplateDir, { recursive: true, force: true }) |
There was a problem hiding this comment.
Temporary template cleanup isn't protected by a try/finally. If create() throws (or the process is interrupted), the generated template-npm-... directory will be left behind in the package root and may accumulate over time. Wrap the npm-template flow in try { ... } finally { rmSync(...) } (and consider removing any pre-existing temp dir before copying).
| // Copy the resolved template to our template directory | |
| const fs = await import('node:fs') | |
| fs.cpSync(templatePath, tempTemplateDir, { recursive: true }) | |
| // Call create with the temporary template | |
| await create({ | |
| root: path.resolve(__dirname, '..'), | |
| name: 'rspeedy', | |
| templates: [templateName], | |
| version: devDependencies, | |
| getTemplateName: async () => templateName, | |
| mapESLintTemplate: () => null, | |
| }) | |
| // Clean up temporary template directory | |
| fs.rmSync(tempTemplateDir, { recursive: true, force: true }) | |
| // Copy the resolved template to our template directory and ensure cleanup | |
| const fs = await import('node:fs') | |
| try { | |
| // Remove any pre-existing temporary template directory before copying | |
| fs.rmSync(tempTemplateDir, { recursive: true, force: true }) | |
| fs.cpSync(templatePath, tempTemplateDir, { recursive: true }) | |
| // Call create with the temporary template | |
| await create({ | |
| root: path.resolve(__dirname, '..'), | |
| name: 'rspeedy', | |
| templates: [templateName], | |
| version: devDependencies, | |
| getTemplateName: async () => templateName, | |
| mapESLintTemplate: () => null, | |
| }) | |
| } finally { | |
| // Clean up temporary template directory even if an error occurs | |
| fs.rmSync(tempTemplateDir, { recursive: true, force: true }) | |
| } |
| // Handle version | ||
| const versionSpecifier = | ||
| version?.trim() && version.trim().toLowerCase() !== 'latest' | ||
| ? version.trim() | ||
| : 'latest' | ||
|
|
||
| // Generate cache key | ||
| const cacheKey = sanitizeCacheKey(normalizedName, versionSpecifier) | ||
| const templateDir = path.join(process.cwd(), '.temp-templates', cacheKey) | ||
| const installRoot = path.dirname(templateDir) | ||
| const packagePath = path.join(installRoot, 'node_modules', normalizedName) | ||
|
|
There was a problem hiding this comment.
versionSpecifier is incorporated into cacheKey/templateDir without sanitization. A malicious or accidental value like --template-version ../outside (or a dist-tag containing path separators) will make path.join(process.cwd(), '.temp-templates', cacheKey) escape the intended cache directory. Sanitize the version string for filesystem usage (similar to packageName.replace(...)) and/or enforce that the resolved templateDir stays within the intended base directory before writing/copying.
| // Install the package | ||
| try { | ||
| execSync( | ||
| `npm install ${normalizedName}@${versionSpecifier} --no-save --package-lock=false --no-audit --no-fund --silent`, | ||
| { | ||
| cwd: installRoot, | ||
| stdio: 'pipe', | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Using npm install to fetch the template will execute that package’s lifecycle scripts (preinstall/install/postinstall) inside .temp-templates/. Since the template source is user-provided, this is an avoidable RCE surface. Prefer fetching/extracting the tarball without running scripts (e.g., npm pack/pacote), or at minimum add --ignore-scripts and document the behavior/tradeoff.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/rspeedy/create-rspeedy/src/template-manager.ts (1)
95-99: Consider preserving original npm error for better debugging.The generic error message loses the original npm error context, making it harder to diagnose issues (e.g., network errors vs. invalid package names).
♻️ Proposed improvement
- } catch { + } catch (error) { + const cause = error instanceof Error ? error.message : String(error) throw new Error( - `Failed to install npm template "${normalizedName}@${versionSpecifier}". Please check if the package exists.`, + `Failed to install npm template "${normalizedName}@${versionSpecifier}": ${cause}`, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rspeedy/create-rspeedy/src/template-manager.ts` around lines 95 - 99, The catch block that throws a new Error for npm install of template "${normalizedName}@${versionSpecifier}" should preserve the original npm error; change the catch to capture the error (e.g., catch (err)) and include the original error message/stack (or set it as the Error cause) when throwing or rethrowing so debugging shows the underlying npm/network error along with the descriptive message about the template install.packages/rspeedy/create-rspeedy/src/index.ts (1)
147-163: Consider wrapping cleanup in try-finally for reliability.If
create()throws an exception, the temporary directory won't be cleaned up. Using try-finally ensures cleanup regardless of success or failure.♻️ Proposed fix
// Copy the resolved template to our template directory const fs = await import('node:fs') fs.cpSync(templatePath, tempTemplateDir, { recursive: true }) - // Call create with the temporary template - await create({ - root: path.resolve(__dirname, '..'), - name: 'rspeedy', - templates: [templateName], - version: devDependencies, - getTemplateName: async () => templateName, - mapESLintTemplate: () => null, - }) - - // Clean up temporary template directory - fs.rmSync(tempTemplateDir, { recursive: true, force: true }) + try { + // Call create with the temporary template + await create({ + root: path.resolve(__dirname, '..'), + name: 'rspeedy', + templates: [templateName], + version: devDependencies, + getTemplateName: async () => templateName, + mapESLintTemplate: () => null, + }) + } finally { + // Clean up temporary template directory + fs.rmSync(tempTemplateDir, { recursive: true, force: true }) + } return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rspeedy/create-rspeedy/src/index.ts` around lines 147 - 163, The temporary template directory is removed only on the happy path, so wrap the call to create(...) in a try-finally block to guarantee cleanup; perform fs.cpSync(templatePath, tempTemplateDir, { recursive: true }) before entering the try, call await create({ ... }) inside the try, and move fs.rmSync(tempTemplateDir, { recursive: true, force: true }) into the finally so tempTemplateDir is always removed even if create() throws (referencing the create function call, tempTemplateDir, templatePath, templateName and the fs import).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/rspeedy/create-rspeedy/README.md`:
- Around line 48-61: The README.md contains a fenced code block showing a
directory tree but missing a language specifier; update the opening fence (the
triple backticks) for that directory diagram to include a language like "text"
or "plaintext" (i.e., change ``` to ```text) so the static analysis tool stops
flagging it and the directory structure (the my-template-package/ tree) is
rendered correctly.
In `@packages/rspeedy/create-rspeedy/src/index.ts`:
- Around line 105-124: The parseArgv function incorrectly uses argv.indexOf(arg)
which returns the first match rather than the current loop index and can point
to the wrong element; replace the for-of loop with an indexed loop (for (let i =
0; i < argv.length; i++)) and use i to read flags and the following value,
validate that argv[i+1] exists before assigning (e.g., when setting
result.templateVersion / result['template-version'], result.template,
result.dir) and handle non-flag positional dir assignment only when result.dir
is unset; ensure flag aliases (--template-version/--tv, --template/-t, --dir/-d)
are processed using the explicit index and skip the consumed value by
incrementing i.
In `@packages/rspeedy/create-rspeedy/src/template-manager.ts`:
- Around line 75-84: The code writes an anchor package.json at anchorPkgJson
(built from installRoot) without ensuring installRoot exists; create the
installRoot directory before calling fs.writeFileSync to avoid ENOENT. Locate
the block that defines installRoot and anchorPkgJson and, when
!fs.existsSync(anchorPkgJson), call a directory creation (e.g.,
fs.mkdirSync(installRoot, { recursive: true }) or an equivalent ensureDir) then
proceed to write the minimal package JSON (the minimal object currently used) to
anchorPkgJson.
- Around line 23-43: The isNpmTemplate function is too permissive and
misclassifies built-in template identifiers as npm packages; update
isNpmTemplate to first check a curated exclusion list of built-in template names
(e.g., react-ts, react-js, etc.) and return false if trimmedInput matches any of
them, then proceed with the existing checks that use NPM_TEMPLATE_PREFIX and the
scoped/package heuristics; reference the isNpmTemplate function and
NPM_TEMPLATE_PREFIX when adding the BUILTIN_TEMPLATES set and ensure the new
membership check occurs before the current npm-detection logic.
---
Nitpick comments:
In `@packages/rspeedy/create-rspeedy/src/index.ts`:
- Around line 147-163: The temporary template directory is removed only on the
happy path, so wrap the call to create(...) in a try-finally block to guarantee
cleanup; perform fs.cpSync(templatePath, tempTemplateDir, { recursive: true })
before entering the try, call await create({ ... }) inside the try, and move
fs.rmSync(tempTemplateDir, { recursive: true, force: true }) into the finally so
tempTemplateDir is always removed even if create() throws (referencing the
create function call, tempTemplateDir, templatePath, templateName and the fs
import).
In `@packages/rspeedy/create-rspeedy/src/template-manager.ts`:
- Around line 95-99: The catch block that throws a new Error for npm install of
template "${normalizedName}@${versionSpecifier}" should preserve the original
npm error; change the catch to capture the error (e.g., catch (err)) and include
the original error message/stack (or set it as the Error cause) when throwing or
rethrowing so debugging shows the underlying npm/network error along with the
descriptive message about the template install.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a15a58e0-dfb2-425f-8fbb-3ec3c73f662e
📒 Files selected for processing (3)
packages/rspeedy/create-rspeedy/README.mdpackages/rspeedy/create-rspeedy/src/index.tspackages/rspeedy/create-rspeedy/src/template-manager.ts
| ``` | ||
| my-template-package/ | ||
| ├── template/ # Preferred | ||
| │ ├── package.json | ||
| │ ├── src/ | ||
| │ └── ... | ||
| ├── templates/ | ||
| │ └── app/ # Alternative | ||
| │ ├── package.json | ||
| │ └── ... | ||
| └── (root) # Fallback | ||
| ├── package.json | ||
| └── ... | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The static analysis tool flagged the missing language identifier. Use text or plaintext for directory structure diagrams.
📝 Proposed fix
-```
+```text
my-template-package/
├── template/ # Preferred🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rspeedy/create-rspeedy/README.md` around lines 48 - 61, The
README.md contains a fenced code block showing a directory tree but missing a
language specifier; update the opening fence (the triple backticks) for that
directory diagram to include a language like "text" or "plaintext" (i.e., change
``` to ```text) so the static analysis tool stops flagging it and the directory
structure (the my-template-package/ tree) is rendered correctly.
| function parseArgv(): ExtendedArgv { | ||
| const argv = process.argv.slice(2) | ||
| const result: ExtendedArgv = {} | ||
|
|
||
| for (const arg of argv) { | ||
| const index = argv.indexOf(arg) | ||
| if (arg === '--template-version' || arg === '--tv') { | ||
| result.templateVersion = argv[index + 1] | ||
| result['template-version'] = result.templateVersion | ||
| } else if (arg === '--template' || arg === '-t') { | ||
| result.template = argv[index + 1] | ||
| } else if (arg === '--dir' || arg === '-d') { | ||
| result.dir = argv[index + 1] | ||
| } else if (!arg.startsWith('-') && !result.dir) { | ||
| result.dir = arg | ||
| } | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| return result | ||
| } |
There was a problem hiding this comment.
Bug: indexOf inside loop returns first occurrence, not current index.
Using argv.indexOf(arg) returns the index of the first occurrence of arg, not the current iteration index. If a value appears multiple times or matches a flag name, this produces incorrect results. Additionally, the code doesn't validate that the next argument exists.
🐛 Proposed fix using explicit index
function parseArgv(): ExtendedArgv {
const argv = process.argv.slice(2)
const result: ExtendedArgv = {}
- for (const arg of argv) {
- const index = argv.indexOf(arg)
+ for (let i = 0; i < argv.length; i++) {
+ const arg = argv[i]
if (arg === '--template-version' || arg === '--tv') {
- result.templateVersion = argv[index + 1]
+ result.templateVersion = argv[i + 1]
result['template-version'] = result.templateVersion
} else if (arg === '--template' || arg === '-t') {
- result.template = argv[index + 1]
+ result.template = argv[i + 1]
} else if (arg === '--dir' || arg === '-d') {
- result.dir = argv[index + 1]
+ result.dir = argv[i + 1]
} else if (!arg.startsWith('-') && !result.dir) {
result.dir = arg
}
}
return result
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function parseArgv(): ExtendedArgv { | |
| const argv = process.argv.slice(2) | |
| const result: ExtendedArgv = {} | |
| for (const arg of argv) { | |
| const index = argv.indexOf(arg) | |
| if (arg === '--template-version' || arg === '--tv') { | |
| result.templateVersion = argv[index + 1] | |
| result['template-version'] = result.templateVersion | |
| } else if (arg === '--template' || arg === '-t') { | |
| result.template = argv[index + 1] | |
| } else if (arg === '--dir' || arg === '-d') { | |
| result.dir = argv[index + 1] | |
| } else if (!arg.startsWith('-') && !result.dir) { | |
| result.dir = arg | |
| } | |
| }, | |
| }) | |
| } | |
| return result | |
| } | |
| function parseArgv(): ExtendedArgv { | |
| const argv = process.argv.slice(2) | |
| const result: ExtendedArgv = {} | |
| for (let i = 0; i < argv.length; i++) { | |
| const arg = argv[i] | |
| if (arg === '--template-version' || arg === '--tv') { | |
| result.templateVersion = argv[i + 1] | |
| result['template-version'] = result.templateVersion | |
| } else if (arg === '--template' || arg === '-t') { | |
| result.template = argv[i + 1] | |
| } else if (arg === '--dir' || arg === '-d') { | |
| result.dir = argv[i + 1] | |
| } else if (!arg.startsWith('-') && !result.dir) { | |
| result.dir = arg | |
| } | |
| } | |
| return result | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rspeedy/create-rspeedy/src/index.ts` around lines 105 - 124, The
parseArgv function incorrectly uses argv.indexOf(arg) which returns the first
match rather than the current loop index and can point to the wrong element;
replace the for-of loop with an indexed loop (for (let i = 0; i < argv.length;
i++)) and use i to read flags and the following value, validate that argv[i+1]
exists before assigning (e.g., when setting result.templateVersion /
result['template-version'], result.template, result.dir) and handle non-flag
positional dir assignment only when result.dir is unset; ensure flag aliases
(--template-version/--tv, --template/-t, --dir/-d) are processed using the
explicit index and skip the consumed value by incrementing i.
| export function isNpmTemplate(templateInput: string): boolean { | ||
| const trimmedInput = templateInput.trim() | ||
|
|
||
| // Explicit npm: prefix | ||
| if (trimmedInput.startsWith(NPM_TEMPLATE_PREFIX)) { | ||
| return true | ||
| } | ||
|
|
||
| // Scoped package (@scope/package) or pure package name (no path separators) | ||
| if ( | ||
| trimmedInput.startsWith('@') | ||
| || (!trimmedInput.includes('/') | ||
| && !trimmedInput.startsWith('http') | ||
| && !trimmedInput.startsWith('.') | ||
| && !trimmedInput.startsWith('github:')) | ||
| ) { | ||
| return true | ||
| } | ||
|
|
||
| return false | ||
| } |
There was a problem hiding this comment.
isNpmTemplate is too permissive and may misclassify built-in template names.
The current logic treats any string without /, not starting with http, ., or github: as an npm template. This means built-in template identifiers like react-ts or react-js would be classified as npm packages, causing the CLI to attempt an npm install that would likely fail.
Consider adding an exclusion list for known built-in templates:
🛠️ Proposed fix
+const BUILTIN_TEMPLATES = new Set(['react-ts', 'react-js', 'react-vitest-rltl-ts', 'react-vitest-rltl-js'])
+
export function isNpmTemplate(templateInput: string): boolean {
const trimmedInput = templateInput.trim()
+ // Exclude known built-in templates
+ if (BUILTIN_TEMPLATES.has(trimmedInput)) {
+ return false
+ }
+
// Explicit npm: prefix
if (trimmedInput.startsWith(NPM_TEMPLATE_PREFIX)) {
return true
}
// ...rest unchanged
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rspeedy/create-rspeedy/src/template-manager.ts` around lines 23 -
43, The isNpmTemplate function is too permissive and misclassifies built-in
template identifiers as npm packages; update isNpmTemplate to first check a
curated exclusion list of built-in template names (e.g., react-ts, react-js,
etc.) and return false if trimmedInput matches any of them, then proceed with
the existing checks that use NPM_TEMPLATE_PREFIX and the scoped/package
heuristics; reference the isNpmTemplate function and NPM_TEMPLATE_PREFIX when
adding the BUILTIN_TEMPLATES set and ensure the new membership check occurs
before the current npm-detection logic.
| // Create isolated package.json to prevent workspace conflicts | ||
| const anchorPkgJson = path.join(installRoot, 'package.json') | ||
| if (!fs.existsSync(anchorPkgJson)) { | ||
| const minimal = { name: 'rspeedy-template-cache', private: true } | ||
| fs.writeFileSync( | ||
| anchorPkgJson, | ||
| `${JSON.stringify(minimal, null, 2)}\n`, | ||
| 'utf8', | ||
| ) | ||
| } |
There was a problem hiding this comment.
Missing directory creation before writing package.json.
If .temp-templates/ doesn't exist on first run, fs.writeFileSync will throw ENOENT because it doesn't create parent directories. The installRoot directory must be created first.
🐛 Proposed fix
// Create isolated package.json to prevent workspace conflicts
const anchorPkgJson = path.join(installRoot, 'package.json')
if (!fs.existsSync(anchorPkgJson)) {
+ fs.mkdirSync(installRoot, { recursive: true })
const minimal = { name: 'rspeedy-template-cache', private: true }
fs.writeFileSync(
anchorPkgJson,
`${JSON.stringify(minimal, null, 2)}\n`,
'utf8',
)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rspeedy/create-rspeedy/src/template-manager.ts` around lines 75 -
84, The code writes an anchor package.json at anchorPkgJson (built from
installRoot) without ensuring installRoot exists; create the installRoot
directory before calling fs.writeFileSync to avoid ENOENT. Locate the block that
defines installRoot and anchorPkgJson and, when !fs.existsSync(anchorPkgJson),
call a directory creation (e.g., fs.mkdirSync(installRoot, { recursive: true })
or an equivalent ensureDir) then proceed to write the minimal package JSON (the
minimal object currently used) to anchorPkgJson.
Add support for using npm packages as templates when creating new Rspeedy projects.
Key Features
npm:,@scope/package,package-name)--template-versionflag for version specification.temp-templates/)template/,templates/app/, root)Usage Examples
Implementation Details
Template Package Structure
The npm template package should have one of the following structures:
Caching Strategy
latestversion are always re-installed to ensure the latest version.temp-templates/for faster reuseTechnical Implementation
isNpmTemplate()function to detect npm package templatesresolveNpmTemplate()to download and extract npm packagesparseArgv()to support--template-versionflagTesting
Related
Inspired by sparkling's npm template implementation.
Checklist
Summary by CodeRabbit
--templateflag.--template-versionoption to specify template versions.