-
Notifications
You must be signed in to change notification settings - Fork 0
Sync files: Many-Repo-Manager-PoC/general-design-system -> Many-Repo-Manager-PoC/mock-repo-with-one-dependency #3
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a Biome.js configuration file and a new package manifest for the design system, defining lint/format settings, VCS integration, file-specific overrides, and an exports map with build/dev scripts and dependencies for an ES module package. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (4)
biome.json (2)
5-13: Align ignore patterns with this repo’s output directoriesThe ignore list still contains a path that looks specific to another repo (“libs/components/lib/*”). Consider ignoring the design-system’s build outputs instead to avoid lint/format work in generated artifacts.
Apply this diff:
"apps/docs/public/pagefind", "lib-types", - "libs/components/lib/*" + "design-system/lib", + "design-system/lib-types"
97-108: Remove repo-specific override for files that likely don’t exist hereThese overrides reference paths from another codebase. Keeping them increases confusion and maintenance burden.
Apply this diff:
}, - { - "include": [ - "apps/docs/src/root.tsx", - "libs/components/src/qr-code/qr-code-canvas.tsx" - ], - "linter": { - "rules": { - "security": { - "noDangerouslySetInnerHtml": "off" - } - } - } - } + // removed overrides referencing non-existent files in this repodesign-system/package.json (2)
5-13: Optional: align or drop “main” in favor of exportsWith a proper “exports” map, many tooling setups ignore “main”. Keeping “main” pointing to ESM while “exports.require” points to CJS can be mildly confusing. Either:
- drop “main” entirely, or
- set “main” to the CJS entry to match older CJS-only consumers.
Not a blocker; just a consistency tweak.
1-4: Meta fields missing (repository, license, homepage)Consider adding “repository”, “license”, and optionally “homepage”/“bugs” for better package metadata on npm.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
biome.json(1 hunks)design-system/package.json(1 hunks)
🔇 Additional comments (4)
biome.json (3)
83-85: Good call enabling organizeImportsAutomatic import sorting helps keep diffs small and code consistent.
111-114: VCS integration looks goodGit-aware behavior from Biome is useful for staged-only runs and better editor UX.
63-70: Rule verified —useSingleCaseStatementis a valid Biome ruleConfirmed via Biome docs:
useSingleCaseStatementexists and enforces a single statement per switch case. No change required — keep the setting as-is.
- File: biome.json (lines ~63–70) — retain
"useSingleCaseStatement": "error"design-system/package.json (1)
9-15: Exports map is well-structuredProviding import/require/types conditions is compatible with both ESM and CJS consumers and TS.
| "suspicious": { | ||
| "noApproximativeNumericConstant": "error", | ||
| "noMisrefactoredShorthandAssign": "warn", | ||
| "recommended": true, | ||
| "noAssignInExpressions": "off" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likely typo in rule name: use noApproximateNumericConstant
The rule key noApproximativeNumericConstant appears misspelled; Biome’s rule is noApproximateNumericConstant. As written, Biome will likely ignore it.
Apply this diff:
- "suspicious": {
- "noApproximativeNumericConstant": "error",
+ "suspicious": {
+ "noApproximateNumericConstant": "error",
"noMisrefactoredShorthandAssign": "warn",
"recommended": true,
"noAssignInExpressions": "off"
},📝 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.
| "suspicious": { | |
| "noApproximativeNumericConstant": "error", | |
| "noMisrefactoredShorthandAssign": "warn", | |
| "recommended": true, | |
| "noAssignInExpressions": "off" | |
| }, | |
| "suspicious": { | |
| "noApproximateNumericConstant": "error", | |
| "noMisrefactoredShorthandAssign": "warn", | |
| "recommended": true, | |
| "noAssignInExpressions": "off" | |
| }, |
🤖 Prompt for AI Agents
In biome.json around lines 71 to 76, the suspicious rules object uses a
misspelled key "noApproximativeNumericConstant"; change that key to the correct
"noApproximateNumericConstant" and keep the same value ("error"), preserving
surrounding JSON structure and commas so the file remains valid.
| "files": ["lib", "lib-types"], | ||
| "engines": { | ||
| "node": "^18.17.0 || ^20.3.0 || >=21.0.0" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Mark CSS as side-effectful to prevent it from being tree-shaken
Because you export a CSS file at “./theme”, bundlers may drop it unless it’s marked as a side effect. Recommend adding a sideEffects entry.
Apply this diff:
"files": ["lib", "lib-types"],
+ "sideEffects": ["./lib/assets/theme.css"],
"engines": {📝 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.
| "files": ["lib", "lib-types"], | |
| "engines": { | |
| "node": "^18.17.0 || ^20.3.0 || >=21.0.0" | |
| }, | |
| "files": ["lib", "lib-types"], | |
| "sideEffects": ["./lib/assets/theme.css"], | |
| "engines": { | |
| "node": "^18.17.0 || ^20.3.0 || >=21.0.0" | |
| }, |
🤖 Prompt for AI Agents
In design-system/package.json around lines 16 to 19, the package exports a CSS
file at "./theme" which bundlers may tree-shake because CSS isn’t declared as
side-effectful; add a "sideEffects" field to package.json that lists the theme
CSS path (or a pattern like "*.css" or "./theme/**") so bundlers know to
preserve it, e.g. add "sideEffects": ["./theme", "./theme.css"] (or a broader
CSS pattern) alongside the existing fields.
| "devDependencies": { | ||
| "@types/node": "22.12", | ||
| "bumpp": "^10.2.0", | ||
| "typescript": "5.4.5", | ||
| "undici": "*", | ||
| "vite": "^7.1", | ||
| "vite-tsconfig-paths": "5.1.4", | ||
| "@qwik.dev/core": "2.0.0-beta.7" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Avoid wildcard version for undici; also verify if it’s needed at all
Using "*" makes builds non-deterministic and can break unexpectedly. If undici isn’t used in the design system tooling, remove it. Otherwise, pin to a compatible semver range (e.g., ^x.y.z).
Run this script to confirm if undici is used anywhere:
- If no usages, remove undici from devDependencies.
- If there are usages, replace "*" with a pinned range appropriate for your Node engines (e.g., ^6 or ^7 depending on compatibility).
🏁 Script executed:
#!/bin/bash
# Find any usage/imports of undici across the repo
rg -n -A2 -B2 "\bundici\b" || trueLength of output: 304
Remove or pin undici in design-system/package.json
rg shows the only occurrence is the declared devDependency (no imports/usages). Wildcard versions make builds non-deterministic — remove it if unused, otherwise pin to a specific semver.
- File to change:
- design-system/package.json — remove or replace the line at/near line 35:
"undici": "*",
- design-system/package.json — remove or replace the line at/near line 35:
Suggested updated snippet (remove undici):
"devDependencies": {
"@types/node": "22.12",
"bumpp": "^10.2.0",
"typescript": "5.4.5",
"vite": "^7.1",
"vite-tsconfig-paths": "5.1.4",
"@qwik.dev/core": "2.0.0-beta.7"
},
Next steps:
- If unused: remove the entry and run your package manager to update the lockfile.
- If required: replace
"*"with an explicit semver range (e.g.,^x.y.z) after confirming compatibility, then update the lockfile.
🤖 Prompt for AI Agents
In design-system/package.json around lines 31 to 39, the devDependency "undici":
"*" is declared with a wildcard and appears unused; remove that entry if the
package is not required and then run your package manager (npm/yarn/pnpm) to
update the lockfile, or if it is required determine a compatible semantic
version and replace "*" with a specific semver range (e.g., ^x.y.z) and then
update the lockfile accordingly.
| "dependencies": { | ||
| "class-variance-authority": "0.7.1", | ||
| "clsx": "2.1.1", | ||
| "tailwind-merge": "3.0.2", | ||
| "tailwindcss": "4.1.8" | ||
| }, | ||
| "peerDependencies": { | ||
| "@tailwindcss/vite": "4.1.8" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Don’t ship Tailwind as a runtime dependency; make it a peer instead
Design-system libraries typically compile CSS during build. Shipping tailwindcss as a runtime dependency inflates install size and can create version conflicts. Prefer making tailwind a peer (or dev-only if not required by consumers at all).
Apply this diff:
"dependencies": {
"class-variance-authority": "0.7.1",
"clsx": "2.1.1",
"tailwind-merge": "3.0.2",
- "tailwindcss": "4.1.8"
+ // moved tailwindcss to peerDependencies
},
"peerDependencies": {
- "@tailwindcss/vite": "4.1.8"
+ "@tailwindcss/vite": "4.1.8",
+ "tailwindcss": "4.1.8"
}If the library truly needs Tailwind at runtime (uncommon), keep as dependency; otherwise move it to devDependencies if only used to build the CSS you publish.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== show design-system/package.json ==="
if [ -f design-system/package.json ]; then
sed -n '1,200p' design-system/package.json || true
else
echo "design-system/package.json not found"
fi
echo
echo "=== rg search for tailwind-related strings in repo ==="
rg --hidden -n --no-ignore -S "tailwindcss|@tailwind|@tailwindcss|tailwind-merge|tailwind" || true
echo
echo "=== show any CSS files containing @tailwind ==="
rg -n --hidden -S "@tailwind" || true
echo
echo "=== show design-system src files (if any) ==="
if [ -d design-system ]; then
rg -n --hidden -S "from 'tailwindcss'|require('tailwindcss')|tailwindcss" design-system || true
rg -n --hidden -S "@tailwind" design-system || true
fi
echo
echo "=== show root package.json (to see workspace setup) ==="
if [ -f package.json ]; then
sed -n '1,200p' package.json || true
else
echo "root package.json not found"
fiLength of output: 2843
Move Tailwind out of runtime dependencies — make it a devDependency
design-system publishes compiled CSS (exports "./lib/assets/theme.css") and there are no @tailwind directives in the repo, so tailwind packages are build-time only. Remove Tailwind from runtime deps and add it to devDependencies.
- Files to change:
- design-system/package.json — remove "tailwindcss" from "dependencies" and remove "@tailwindcss/vite" from "peerDependencies"; add both to "devDependencies".
Apply this patch:
--- design-system/package.json
+++ design-system/package.json
@@
"devDependencies": {
"@types/node": "22.12",
"bumpp": "^10.2.0",
"typescript": "5.4.5",
"undici": "*",
"vite": "^7.1",
"vite-tsconfig-paths": "5.1.4",
- "@qwik.dev/core": "2.0.0-beta.7"
+ "@qwik.dev/core": "2.0.0-beta.7",
+ "tailwindcss": "4.1.8",
+ "@tailwindcss/vite": "4.1.8"
},
"dependencies": {
"class-variance-authority": "0.7.1",
"clsx": "2.1.1",
"tailwind-merge": "3.0.2",
- "tailwindcss": "4.1.8"
},
- "peerDependencies": {
- "@tailwindcss/vite": "4.1.8"
- }
+ "peerDependencies": {}If consumers are expected to compile Tailwind themselves (rare), consider making Tailwind a peer instead; otherwise keep it as a devDependency.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In design-system/package.json around lines 40 to 48, Tailwind is incorrectly
listed as a runtime dependency and its Vite plugin as a peer; remove
"tailwindcss" from the "dependencies" block and remove "@tailwindcss/vite" from
"peerDependencies", then add both "tailwindcss" and "@tailwindcss/vite" to
"devDependencies" with appropriate versions (move the existing versions over),
ensuring no other runtime references remain and package.json remains valid JSON.
Summary by CodeRabbit
New Features
Chores