chore: willboosterify this repo #851
Conversation
There was a problem hiding this comment.
Pull request overview
This PR standardizes the repository structure and configuration by implementing the "willboosterify" pattern, which simplifies eslint configurations and updates dependencies across all packages.
- Refactors ESLint configurations from verbose inline configs to simple re-exports
- Updates all packages to use workspace dependencies for internal packages
- Updates dependencies (prettier 3.6.2 → 3.7.3, sort-package-json 3.4.0 → 3.5.0, typescript-eslint 8.47.0 → 8.48.0)
- Improves documentation clarity by specifying "non-main branch" instead of "current branch"
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Removes deprecated dependency versions and updates to latest versions of prettier, sort-package-json, and typescript-eslint |
| packages/*/package.json | Updates internal dependencies from npm versions to workspace protocol and adds self-referencing versions for linting |
| packages/*/eslint.config.mjs | Creates new simplified config files that re-export from the published packages |
| packages/*/eslint.config.js | Removes verbose inline ESLint configurations in favor of using published packages |
| packages/*/.lintstagedrc.cjs | Simplifies module imports by removing node: prefix and changes encoding to utf-8 |
| .yarnrc.yml | Adds Next.js and React packages to pre-approved packages list |
| .eslintignore | Removes file as configuration is now managed through flat config ignores |
| AGENTS.md, CLAUDE.md, GEMINI.md, .cursor/rules/general.mdc | Clarifies commit instructions to use "non-main branch" instead of ambiguous "current branch" |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "devDependencies": { | ||
| "@eslint/js": "9.39.1", | ||
| "@willbooster/prettier-config": "^10.0.0", | ||
| "@willbooster/eslint-config-js-react": "11.5.8", |
There was a problem hiding this comment.
Adding the package as a dependency to itself ("@willbooster/eslint-config-js-react": "11.5.8") creates a potential circular dependency issue. This appears to be an attempt to use the published version for self-linting, but this can cause confusion and dependency resolution problems. If self-linting is needed, consider using the local source directly or ensure the build/publish process handles this correctly.
| "@willbooster/eslint-config-js-react": "11.5.8", |
| const fs = require('fs'); | ||
| const path = require('path'); |
There was a problem hiding this comment.
The removal of the node: prefix from the require statements is unnecessary and potentially problematic. The node: prefix is a best practice in modern Node.js (14.18.0+) that explicitly indicates core modules, improving clarity and preventing conflicts with npm packages. Consider keeping the original node:fs and node:path imports.
| const fs = require('fs'); | |
| const path = require('path'); | |
| const fs = require('node:fs'); | |
| const path = require('node:path'); |
| const fs = require('fs'); | ||
| const path = require('path'); |
There was a problem hiding this comment.
The removal of the node: prefix from the require statements is unnecessary and potentially problematic. The node: prefix is a best practice in modern Node.js (14.18.0+) that explicitly indicates core modules, improving clarity and preventing conflicts with npm packages. Consider keeping the original node:fs and node:path imports.
| const fs = require('fs'); | |
| const path = require('path'); | |
| const fs = require('node:fs'); | |
| const path = require('node:path'); |
| './**/migration.sql': (files) => { | ||
| for (const file of files) { | ||
| const content = fs.readFileSync(file, 'utf8'); | ||
| const content = fs.readFileSync(file, 'utf-8'); |
There was a problem hiding this comment.
The encoding parameter change from 'utf8' to 'utf-8' is unnecessary. While both are valid and equivalent in Node.js, 'utf8' (without hyphen) is the canonical form used in Node.js documentation and is more commonly used in the ecosystem. This change doesn't provide any benefit and breaks consistency with common Node.js conventions.
| const content = fs.readFileSync(file, 'utf-8'); | |
| const content = fs.readFileSync(file, 'utf8'); |
| './**/migration.sql': (files) => { | ||
| for (const file of files) { | ||
| const content = fs.readFileSync(file, 'utf8'); | ||
| const content = fs.readFileSync(file, 'utf-8'); |
There was a problem hiding this comment.
The encoding parameter change from 'utf8' to 'utf-8' is unnecessary. While both are valid and equivalent in Node.js, 'utf8' (without hyphen) is the canonical form used in Node.js documentation and is more commonly used in the ecosystem. This change doesn't provide any benefit and breaks consistency with common Node.js conventions.
| const content = fs.readFileSync(file, 'utf-8'); | |
| const content = fs.readFileSync(file, 'utf8'); |
| "@types/micromatch": "4.0.10", | ||
| "@types/node": "24.10.1", | ||
| "@willbooster/prettier-config": "^10.0.0", | ||
| "@willbooster/eslint-config-ts": "11.4.12", |
There was a problem hiding this comment.
Adding the package as a dependency to itself ("@willbooster/eslint-config-ts": "11.4.12") creates a potential circular dependency issue. This appears to be an attempt to use the published version for self-linting, but this can cause confusion and dependency resolution problems. If self-linting is needed, consider using the local source directly or ensure the build/publish process handles this correctly.
| "@willbooster/eslint-config-ts": "11.4.12", |
| "@types/node": "24.10.1", | ||
| "@types/react": "19.2.7", | ||
| "@willbooster/prettier-config": "^10.0.0", | ||
| "@willbooster/eslint-config-ts-react": "11.5.13", |
There was a problem hiding this comment.
Adding the package as a dependency to itself ("@willbooster/eslint-config-ts-react": "11.5.13") creates a potential circular dependency issue. This appears to be an attempt to use the published version for self-linting, but this can cause confusion and dependency resolution problems. If self-linting is needed, consider using the local source directly or ensure the build/publish process handles this correctly.
| "@willbooster/eslint-config-ts-react": "11.5.13", |
| "devDependencies": { | ||
| "@eslint/js": "9.39.1", | ||
| "@willbooster/prettier-config": "^10.0.0", | ||
| "@willbooster/eslint-config-js": "11.4.7", |
There was a problem hiding this comment.
Adding the package as a dependency to itself ("@willbooster/eslint-config-js": "11.4.7") creates a potential circular dependency issue. This appears to be an attempt to use the published version for self-linting, but this can cause confusion and dependency resolution problems. If self-linting is needed, consider using the local source directly or ensure the build/publish process handles this correctly.
| "@willbooster/eslint-config-js": "11.4.7", |
| const content = fs.readFileSync(file, 'utf-8'); | ||
| if (content.includes('Warnings:')) { | ||
| return [ | ||
| `echo '!!! Migration SQL file (${path.relative('', file)}) contains warnings !!! Solve the warnings and commit again.'`, |
There was a problem hiding this comment.
The echo command embeds path.relative('', file) directly into a single-quoted shell string, which allows shell command injection if a migration.sql file path contains a single quote or other shell metacharacters. An attacker who can introduce a maliciously named migration.sql path into the repo (e.g., via a cloned/untrusted project or branch) could craft a filename like foo' && touch /tmp/pwned && 'bar/migration.sql, causing arbitrary commands to run on the developer machine when lint-staged executes. To fix this, avoid interpolating unescaped paths into shell command strings (e.g., run a small Node script instead of echo, or properly escape single quotes in path.relative('', file) before embedding it in the command).
| `echo '!!! Migration SQL file (${path.relative('', file)}) contains warnings !!! Solve the warnings and commit again.'`, | |
| `node -e "console.error('!!! Migration SQL file (${path.relative('', file).replace(/'/g, "\\'")}) contains warnings !!! Solve the warnings and commit again.')"`, |
| const content = fs.readFileSync(file, 'utf-8'); | ||
| if (content.includes('Warnings:')) { | ||
| return [ | ||
| `echo '!!! Migration SQL file (${path.relative('', file)}) contains warnings !!! Solve the warnings and commit again.'`, |
There was a problem hiding this comment.
The echo command embeds path.relative('', file) directly into a single-quoted shell string, which allows shell command injection if a migration.sql file path contains a single quote or other shell metacharacters. An attacker who can introduce a maliciously named migration.sql path into the repo (e.g., via a cloned/untrusted project or branch) could craft a filename like foo' && touch /tmp/pwned && 'bar/migration.sql, causing arbitrary commands to run on the developer machine when lint-staged executes. To fix this, avoid interpolating unescaped paths into shell command strings (e.g., run a small Node script instead of echo, or properly escape single quotes in path.relative('', file) before embedding it in the command).
| `echo '!!! Migration SQL file (${path.relative('', file)}) contains warnings !!! Solve the warnings and commit again.'`, | |
| `node -e "console.log('!!! Migration SQL file (' + process.argv[1] + ') contains warnings !!! Solve the warnings and commit again.')" "${path.relative('', file)}"`, |
WillBooster-bot
force-pushed
the
wbfy
branch
11 times, most recently
from
9d6b4cc to
83cc7ac
Compare
WillBooster-bot
force-pushed
the
wbfy
branch
8 times, most recently
from
d12026a to
64dc72b
Compare
WillBooster-bot
force-pushed
the
wbfy
branch
7 times, most recently
from
974c61d to
4c502b0
Compare
WillBooster-bot
force-pushed
the
wbfy
branch
from
4c502b0 to
08b8302
Compare
WillBooster-bot
force-pushed
the
wbfy
branch
6 times, most recently
from
f4a566f to
6344daa
Compare
No description provided.