Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a .planning documentation tree, updates package metadata/dependencies, and threads a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (user)
participant Orchestrator as build.ts
participant Prettier as build-prettier
participant Linter as build-eslint
participant TSC as build-tsc
participant Bundler as build-bundle
participant FS as File System / Console
CLI->>Orchestrator: run build (--silent? true/false)
Orchestrator->>Prettier: prettifySrc(..., silent)
Prettier->>FS: (log start/end if !silent)
Orchestrator->>Linter: lint(ci, silent)
Linter->>FS: (log start/end if !silent)
Orchestrator->>TSC: buildTS(basePath, silent)
TSC->>FS: (log progress if !silent)
Orchestrator->>Bundler: bundle(basePath, silent)
Bundler->>FS: (log start/end if !silent)
Orchestrator->>FS: emit final summary (only if !silent)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 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 |
There was a problem hiding this comment.
Pull request overview
This PR bumps the @tsparticles/cli package from version 3.1.7 to 3.2.0. The main change introduces a new --silent (-s) flag for the build command that reduces the amount of output during builds. When --ci is used, silent defaults to true. Several dependencies are also updated to their latest minor/patch versions.
Changes:
- Added
--silentflag to the build command and propagated it to all build sub-functions (clear, prettier, eslint, tsc, circular-deps, bundle, distfiles) - Updated several dependencies to newer patch/minor versions (
@swc/core,webpack,eslint-plugin-tsdoc,@types/node, etc.) and updatedpnpm-lock.yamlaccordingly - Added
.planning/directory with auto-generated planning placeholder files
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/build/build.ts |
Adds --silent CLI option and propagates the flag to all sub-build function calls |
src/build/build-tsc.ts |
Accepts and respects silent parameter to suppress log output |
src/build/build-prettier.ts |
Accepts and respects silent parameter across all prettier sub-functions |
src/build/build-eslint.ts |
Accepts and respects silent parameter for lint output |
src/build/build-distfiles.ts |
Accepts and respects silent parameter for dist file build output |
src/build/build-clear.ts |
Accepts and respects silent parameter for dist clearing output |
src/build/build-circular-deps.ts |
Accepts and respects silent parameter for circular dependency check output |
src/build/build-bundle.ts |
Accepts and respects silent parameter for webpack bundling output |
package.json |
Version bump to 3.2.0, dependency version updates |
pnpm-lock.yaml |
Lockfile updated to reflect new dependency versions |
files/empty-project/package.json |
Updated template references to match new @tsparticles/cli and @swc/core versions |
.planning/ files |
Auto-generated planning artifacts with placeholder content (should not be committed) |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
.planning/codebase/STACK.md (1)
3-11: Nice stack overview; consider adding version targets.Including expected Node/TypeScript/pnpm version ranges would make this doc more actionable for onboarding and CI consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/codebase/STACK.md around lines 3 - 11, Add explicit version targets to the stack doc: for each listed runtime/tool (Node.js, TypeScript, pnpm, Vitest, tsc) state the supported version or semver range (e.g., Node.js >=16.17.0 <20, TypeScript ^5.0, pnpm >=8), and note where they are enforced (e.g., package.json engines, .nvmrc, or CI matrix). Update the "Languages & Runtimes" and "Tools" sections to include these ranges and add a short line under "Config locations" describing which file pins versions (e.g., engines in package.json, tsconfig.json target)..planning/research/STACK.md (1)
1-1: Replace placeholder with actual stack research notes before merge.Line 1 is still placeholder text, so this file doesn’t yet support planning decisions.
Proposed content skeleton
- STACK research placeholder + # Stack Research + + ## Current stack + - Runtime: + - Language: + - Build/Test tooling: + + ## Alternatives considered + - Option A: pros/cons + - Option B: pros/cons + + ## Decision + - Chosen approach: + - Rationale: + - Risks / mitigations:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/research/STACK.md at line 1, Replace the placeholder text "STACK research placeholder" in STACK.md with substantive stack research notes: summarize chosen technologies, rationale, tradeoffs, integration points, deployment considerations, and any open decisions; populate the proposed content skeleton (architecture overview, component list, pros/cons, migration risks, and references) so the file is ready for planning and review and remove the placeholder line..planning/research/ARCHITECTURE.md (1)
1-1: Architecture research file needs real content, not placeholder text.At minimum, add system boundaries, major components, and key trade-offs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/research/ARCHITECTURE.md at line 1, Replace the placeholder "ARCH research placeholder" in .planning/research/ARCHITECTURE.md with real architecture content: add a "System Boundaries" section that lists external actors and APIs, a "Major Components" section describing each component/module (e.g., API Gateway, Auth service, Data Layer, Worker/Queue, Frontend) and their responsibilities, and a "Key Trade-offs" section that explains important design decisions (scalability vs. cost, consistency vs. availability, technology choices) including rationale and alternatives; ensure each section has concise bullets and diagrams or ASCII boxes if helpful so reviewers can quickly understand ownership and interactions..planning/codebase/CONCERNS.md (1)
1-3: Make concern items trackable.Good items, but they should include owner/severity/status to drive follow-through.
Suggested structure
Concerns -- Some build scripts may lack tests -- CI secrets should be reviewed +- [ ] Build scripts may lack tests (owner: , severity: , status: ) +- [ ] CI secrets review pending (owner: , severity: , status: )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/codebase/CONCERNS.md around lines 1 - 3, Update the "Concerns" list to make each item trackable by replacing plain bullets with a structured entry template that includes Owner, Severity (e.g., Low/Med/High), Status (e.g., Open/In Progress/Resolved), and optional Notes/Created date; specifically convert the existing items "Some build scripts may lack tests" and "CI secrets should be reviewed" into that template so each concern has an explicit Owner, Severity and Status, and add a short instruction line at the top (under the "Concerns" header) describing the required fields and how to update them going forward..planning/research/FEATURES.md (1)
1-1: This research doc is still placeholder-only.Please replace Line 1 with scoped feature research (candidate features, constraints, and priority) so the file is actionable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/research/FEATURES.md at line 1, Replace the placeholder line "FEATURES research placeholder" with a scoped research summary that lists candidate features, constraints, and priority; specifically include a short header (e.g., "FEATURES — Research Summary"), a bullet-like compact list of candidate features with one-line descriptions, a constraints section (technical, legal, timeline), and a prioritized ranking (High/Medium/Low) so the file is actionable and replaces the existing placeholder text.src/build/build-eslint.ts (1)
36-36: ESLint result output is not gated bysilentflag.The
console.log(resultText)on line 36 outputs the formatted ESLint results regardless of thesilentparameter. This differs from other build modules where informational output is suppressed in silent mode.If this is intentional (always show lint results even in silent mode), consider adding a comment to clarify. Otherwise, wrap it with the silent check for consistency.
♻️ Optional: Gate output with silent flag for consistency
- console.log(resultText); + if (!silent) { + console.log(resultText); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/build/build-eslint.ts` at line 36, The console.log(resultText) unconditionally prints ESLint output; update the build-eslint module to respect the existing silent parameter by only logging resultText when silent is false (or add a clear comment if always-on logging is intentional). Locate the console.log(resultText) and the function or scope that receives the silent parameter (reference the silent variable and resultText identifiers) and wrap the log call in a conditional (if (!silent) { ... }) or add an inline comment explaining why output must always be shown..planning/codebase/ARCHITECTURE.md (1)
1-3: Add Markdown heading syntax.The title should use proper Markdown heading format for consistency with other planning documents.
📝 Proposed fix
-Architecture Overview +# Architecture Overview + - CLI entrypoint: src/cli.ts - Feature modules: src/create, src/build🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/codebase/ARCHITECTURE.md around lines 1 - 3, Change the plain title "Architecture Overview" to a Markdown heading by prepending the appropriate heading marker (e.g., "# Architecture Overview") so the document matches other planning files; update the top line containing the exact string "Architecture Overview" to use the Markdown heading syntax while leaving the rest of the list items ("CLI entrypoint: src/cli.ts", "Feature modules: src/create, src/build") intact.src/build/build-prettier.ts (1)
206-258: Consider extracting shared prettier options.The prettier options (
printWidth: 120,endOfLine: "lf",tabWidth: 2) are duplicated across multiple functions. Consider extracting a shared base configuration object to reduce repetition.♻️ Optional: Extract shared prettier options
const basePrettierOptions = { printWidth: 120, endOfLine: "lf" as const, tabWidth: 2, }; // Then in each function: const options = { ...basePrettierOptions, ...(await prettier.resolveConfig(basePath)) ?? {}, parser: "markdown" };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/build/build-prettier.ts` around lines 206 - 258, The prettier options used in prettifyTraductions are duplicated elsewhere; extract a shared constant (e.g., basePrettierOptions) containing printWidth:120, endOfLine:"lf", tabWidth:2 and then merge it with the resolved config inside prettifyTraductions (and other prettify* functions) so options = { ...basePrettierOptions, ...(await prettier.resolveConfig(basePath)) ?? {}, parser: "markdown" }; update prettifyTraductions to use that merged options object when calling prettier.check/format so the duplicated literals are removed and consistency is preserved..planning/REQUIREMENTS.md (1)
3-3: Replace placeholder dates.The
[date]placeholders on lines 3 and 41 should be filled in with actual dates or removed if not needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/REQUIREMENTS.md at line 3, Replace the placeholder "[date]" entries in .planning/REQUIREMENTS.md (occurrences at lines containing **Defined:** [date] and the other [date] on line 41) with actual dates or remove the placeholder if no date is required; ensure the replaced text follows the surrounding format (e.g., "**Defined:** YYYY-MM-DD" or plain text) so the document no longer contains "[date]" tokens..planning/PROJECT.md (1)
43-43: Replace placeholder date.The
[date]placeholder should be filled in with the actual date or use an auto-updating mechanism.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.planning/PROJECT.md at line 43, Replace the literal placeholder "[date]" in the markdown line "_Last updated: [date]_" with a real date (e.g., ISO format "YYYY-MM-DD") or wire up an auto-update mechanism (e.g., a CI/GitHub Action that injects/updates the last-updated value) so the "_Last updated: [date]_" line contains a concrete, current date instead of the placeholder.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/build/build.ts`:
- Around line 22-26: The Commander option for silent is defined incorrectly
using a value placeholder; update the buildCommand.option call that currently
uses "-s, --silent <boolean>" to a proper boolean flag (e.g., "-s, --silent") so
the option does not parse a string value, and ensure uses of
opts["silent"]/!!opts["silent"] (in the build flow) will reflect the actual
boolean flag behavior and default logic (including CI overrides) rather than
treating strings like "false" as truthy.
---
Nitpick comments:
In @.planning/codebase/ARCHITECTURE.md:
- Around line 1-3: Change the plain title "Architecture Overview" to a Markdown
heading by prepending the appropriate heading marker (e.g., "# Architecture
Overview") so the document matches other planning files; update the top line
containing the exact string "Architecture Overview" to use the Markdown heading
syntax while leaving the rest of the list items ("CLI entrypoint: src/cli.ts",
"Feature modules: src/create, src/build") intact.
In @.planning/codebase/CONCERNS.md:
- Around line 1-3: Update the "Concerns" list to make each item trackable by
replacing plain bullets with a structured entry template that includes Owner,
Severity (e.g., Low/Med/High), Status (e.g., Open/In Progress/Resolved), and
optional Notes/Created date; specifically convert the existing items "Some build
scripts may lack tests" and "CI secrets should be reviewed" into that template
so each concern has an explicit Owner, Severity and Status, and add a short
instruction line at the top (under the "Concerns" header) describing the
required fields and how to update them going forward.
In @.planning/codebase/STACK.md:
- Around line 3-11: Add explicit version targets to the stack doc: for each
listed runtime/tool (Node.js, TypeScript, pnpm, Vitest, tsc) state the supported
version or semver range (e.g., Node.js >=16.17.0 <20, TypeScript ^5.0, pnpm
>=8), and note where they are enforced (e.g., package.json engines, .nvmrc, or
CI matrix). Update the "Languages & Runtimes" and "Tools" sections to include
these ranges and add a short line under "Config locations" describing which file
pins versions (e.g., engines in package.json, tsconfig.json target).
In @.planning/PROJECT.md:
- Line 43: Replace the literal placeholder "[date]" in the markdown line "_Last
updated: [date]_" with a real date (e.g., ISO format "YYYY-MM-DD") or wire up an
auto-update mechanism (e.g., a CI/GitHub Action that injects/updates the
last-updated value) so the "_Last updated: [date]_" line contains a concrete,
current date instead of the placeholder.
In @.planning/REQUIREMENTS.md:
- Line 3: Replace the placeholder "[date]" entries in .planning/REQUIREMENTS.md
(occurrences at lines containing **Defined:** [date] and the other [date] on
line 41) with actual dates or remove the placeholder if no date is required;
ensure the replaced text follows the surrounding format (e.g., "**Defined:**
YYYY-MM-DD" or plain text) so the document no longer contains "[date]" tokens.
In @.planning/research/ARCHITECTURE.md:
- Line 1: Replace the placeholder "ARCH research placeholder" in
.planning/research/ARCHITECTURE.md with real architecture content: add a "System
Boundaries" section that lists external actors and APIs, a "Major Components"
section describing each component/module (e.g., API Gateway, Auth service, Data
Layer, Worker/Queue, Frontend) and their responsibilities, and a "Key
Trade-offs" section that explains important design decisions (scalability vs.
cost, consistency vs. availability, technology choices) including rationale and
alternatives; ensure each section has concise bullets and diagrams or ASCII
boxes if helpful so reviewers can quickly understand ownership and interactions.
In @.planning/research/FEATURES.md:
- Line 1: Replace the placeholder line "FEATURES research placeholder" with a
scoped research summary that lists candidate features, constraints, and
priority; specifically include a short header (e.g., "FEATURES — Research
Summary"), a bullet-like compact list of candidate features with one-line
descriptions, a constraints section (technical, legal, timeline), and a
prioritized ranking (High/Medium/Low) so the file is actionable and replaces the
existing placeholder text.
In @.planning/research/STACK.md:
- Line 1: Replace the placeholder text "STACK research placeholder" in STACK.md
with substantive stack research notes: summarize chosen technologies, rationale,
tradeoffs, integration points, deployment considerations, and any open
decisions; populate the proposed content skeleton (architecture overview,
component list, pros/cons, migration risks, and references) so the file is ready
for planning and review and remove the placeholder line.
In `@src/build/build-eslint.ts`:
- Line 36: The console.log(resultText) unconditionally prints ESLint output;
update the build-eslint module to respect the existing silent parameter by only
logging resultText when silent is false (or add a clear comment if always-on
logging is intentional). Locate the console.log(resultText) and the function or
scope that receives the silent parameter (reference the silent variable and
resultText identifiers) and wrap the log call in a conditional (if (!silent) {
... }) or add an inline comment explaining why output must always be shown.
In `@src/build/build-prettier.ts`:
- Around line 206-258: The prettier options used in prettifyTraductions are
duplicated elsewhere; extract a shared constant (e.g., basePrettierOptions)
containing printWidth:120, endOfLine:"lf", tabWidth:2 and then merge it with the
resolved config inside prettifyTraductions (and other prettify* functions) so
options = { ...basePrettierOptions, ...(await prettier.resolveConfig(basePath))
?? {}, parser: "markdown" }; update prettifyTraductions to use that merged
options object when calling prettier.check/format so the duplicated literals are
removed and consistency is preserved.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
.planning/PROJECT.md.planning/REQUIREMENTS.md.planning/ROADMAP.md.planning/STATE.md.planning/codebase/ARCHITECTURE.md.planning/codebase/CONCERNS.md.planning/codebase/CONVENTIONS.md.planning/codebase/INTEGRATIONS.md.planning/codebase/STACK.md.planning/codebase/STRUCTURE.md.planning/codebase/TESTING.md.planning/config.json.planning/research/ARCHITECTURE.md.planning/research/FEATURES.md.planning/research/PITFALLS.md.planning/research/STACK.md.planning/research/SUMMARY.mdfiles/empty-project/package.jsonpackage.jsonsrc/build/build-bundle.tssrc/build/build-circular-deps.tssrc/build/build-clear.tssrc/build/build-distfiles.tssrc/build/build-eslint.tssrc/build/build-prettier.tssrc/build/build-tsc.tssrc/build/build.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/build/build.ts (1)
22-26:⚠️ Potential issue | 🟠 MajorUse a real boolean Commander flag for
silent(current parsing is brittle).
"-s, --silent <boolean>"makes Commander parse a string value, then manual coercion at Lines 43-44 has edge cases and confusing UX. This is the same issue previously reported and still present.🔧 Proposed fix
buildCommand.option( - "-s, --silent <boolean>", + "-s, --silent", "Reduce the amount of output during the build, defaults to false, except when --ci is set", - false, ); @@ - silentOpt = opts["silent"] as string | boolean, - silent = silentOpt === "false" ? false : !!silentOpt || ci, + silent = !!opts["silent"] || ci,Commander.js docs: confirm whether "-s, --silent <boolean>" parses a string argument and whether "-s, --silent" is the correct syntax for a boolean flag.Also applies to: 43-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/build/build.ts` around lines 22 - 26, Change the `silent` option to a real boolean flag by updating the call to buildCommand.option to use the flag form (remove the "<boolean>" token) so Commander treats it as a boolean (e.g., "-s, --silent" with default false), then remove the brittle string-to-boolean coercion logic that currently inspects the parsed value (the manual parsing at the code that reads opts.silent / parsedOptions.silent around Lines 43-44); instead rely on Commander to produce a boolean and only apply the existing CI-related default logic (where --ci forces silent) by checking the boolean opts.ci and opts.silent values (and setting opts.silent = true when opts.ci is true) so all code that references the silent flag (opts.silent, parsedOptions.silent, buildCommand.option for silent) uses a proper boolean.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/build/build.ts`:
- Around line 22-26: Change the `silent` option to a real boolean flag by
updating the call to buildCommand.option to use the flag form (remove the
"<boolean>" token) so Commander treats it as a boolean (e.g., "-s, --silent"
with default false), then remove the brittle string-to-boolean coercion logic
that currently inspects the parsed value (the manual parsing at the code that
reads opts.silent / parsedOptions.silent around Lines 43-44); instead rely on
Commander to produce a boolean and only apply the existing CI-related default
logic (where --ci forces silent) by checking the boolean opts.ci and opts.silent
values (and setting opts.silent = true when opts.ci is true) so all code that
references the silent flag (opts.silent, parsedOptions.silent,
buildCommand.option for silent) uses a proper boolean.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
files/empty-project/package.jsonpackage.jsonsrc/build/build.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- files/empty-project/package.json
- package.json
Summary by CodeRabbit
New Features
Chores