Patch Vite dependency and document metadata parser risk#16
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates Vite and related build-time dependencies to the latest 4.5.x patch and refreshes the lockfile, while documenting ongoing risk around the deprecated music-metadata-browser dependency for manual QA. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider moving build-time-only packages like
@types/*,@vitejs/plugin-react,autoprefixer,postcss,tailwindcss,typescript, and possiblyviteintodevDependenciesto better reflect their usage and avoid inflating the production dependency surface.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider moving build-time-only packages like `@types/*`, `@vitejs/plugin-react`, `autoprefixer`, `postcss`, `tailwindcss`, `typescript`, and possibly `vite` into `devDependencies` to better reflect their usage and avoid inflating the production dependency surface.
## Individual Comments
### Comment 1
<location path="package.json" line_range="19-28" />
<code_context>
"dependencies": {
"@tokenizer/http": "^0.9.2",
"@tokenizer/range": "^0.13.1",
+ "@types/react": "^18.3.3",
+ "@types/react-dom": "^18.3.0",
+ "@vitejs/plugin-react": "^4.2.1",
</code_context>
<issue_to_address>
**suggestion (performance):** Consider keeping tooling and type packages in devDependencies instead of dependencies.
These packages are only required at build time, so they should live under `devDependencies` (and the `devDependencies` section should be reintroduced). This keeps the production install smaller and clearly distinguishes runtime from build-time dependencies, which is especially helpful for lean production images and functions.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "@types/react": "^18.3.3", | ||
| "@types/react-dom": "^18.3.0", | ||
| "@vitejs/plugin-react": "^4.2.1", | ||
| "autoprefixer": "^10.4.19", | ||
| "bcryptjs": "^2.4.3", | ||
| "better-sqlite3": "^9.6.0", | ||
| "browser-id3-writer": "4.4.0", | ||
| "cors": "^2.8.5", | ||
| "dotenv": "^16.4.5", | ||
| "exiftool-vendored": "^28.3.1", |
There was a problem hiding this comment.
suggestion (performance): Consider keeping tooling and type packages in devDependencies instead of dependencies.
These packages are only required at build time, so they should live under devDependencies (and the devDependencies section should be reintroduced). This keeps the production install smaller and clearly distinguishes runtime from build-time dependencies, which is especially helpful for lean production images and functions.
Motivation
esbuildexposure without introducing breaking changes.music-metadata-browserdependency for manual QA without replacing it in this PR.Description
vitefrom^4.5.3to^4.5.14inpackage.jsonand regeneratedpackage-lock.jsonto capture updated transitive versions.docs/manual-qa-checklist.mddocumenting thatmusic-metadata-browseris deprecated, used only for browser-side metadata analysis, should be replaced/redesigned in a future PR, and that suspicious/corrupt media requires careful QA.package.json,package-lock.json, and the QA docs.music-metadata-browserbecause no safe, non-breaking drop-in was available within the scope and risk constraints of this patch.Testing
npm audit --jsonbefore and after the Vite update; vulnerability counts remained5 total(3 moderate,2 high) and theviteand transitiveesbuildadvisories still appear (no resolution by the patch).npm installsuccessfully and regeneratedpackage-lock.jsonto reflectvite@4.5.14and its transitive dependencies.npm run buildsuccessfully (production build completes; warning aboutfile-typeeval usage noted but build artifacts produced).npm outdated || trueto surface newer majors; command executed successfully and no further upgrades were applied in this PR.Codex Task
Summary by Sourcery
Update Vite to the latest 4.5.x patch and document QA risks around the deprecated music-metadata-browser dependency.
Enhancements:
Build:
Documentation: