feat: migrate OG generation to takumi-js v2#1008
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR replaces Takumi dependencies with ChangesTakumi migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
|
@AlemTuzlak please take a look at this follow up PR if you could thanks! Btw you can go a step further if you want to try the new on-demand google fonts feature to get rid of those static font assets |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
107-107: 📐 Maintainability & Code Quality | 🔵 TrivialPin the beta version exactly to ensure reproducible builds.
The caret range
^2.0.0-beta.10allows NPM to install any version from2.0.0-beta.10up to<3.0.0. Sincenpmdefaults to thelatestdist-tag (version1.8.7) and may skip over specific pre-release versions unless explicitly targeted or updated to newer betas (e.g.,2.0.0-beta.11) if available, using a range with unstable pre-1.0 releases risks introducing breaking changes during standard installs. Pin the version to maintain consistency.📌 Proposed change
- "takumi-js": "^2.0.0-beta.10", + "takumi-js": "2.0.0-beta.10",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 107, The takumi-js dependency is using a caret beta range, which can vary across installs and reduce build reproducibility. Update the dependency declaration in package.json to pin takumi-js exactly to the current beta version instead of using the ^2.0.0-beta.10 range, keeping the package specification stable for deterministic installs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@package.json`:
- Line 107: The takumi-js dependency is using a caret beta range, which can vary
across installs and reduce build reproducibility. Update the dependency
declaration in package.json to pin takumi-js exactly to the current beta version
instead of using the ^2.0.0-beta.10 range, keeping the package specification
stable for deterministic installs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 781fc453-203f-4492-bfe1-8e8f24c806cd
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
package.jsonsrc/server/og/generate.server.tsvite.config.ts
💤 Files with no reviewable changes (1)
- vite.config.ts
Drop the @takumi-rs/* v1 packages for takumi-js 2.0.0-beta.10. The v2 package picks its backend from `#backend` import conditions, so workerd resolves to wasm automatically — removing the manual vite plugin, alias, and optimizeDeps plumbing that forced the wasm runtime. Rename `persistentImages` to `images` per the v2 ImageResponse API. Claude-Session: https://claude.ai/code/session_01MvKWysun1A8RCvfuvqyYiJ
cd1aa16 to
672fe1b
Compare
|
@kane50613 looks good, before i merge I just wanted to clarify for myself, why is the wasm hack not required anymore? |
Cuz there was an accidental So now Vite SSR with workers plugin should resolves correctly, I tested locally but not sure if it can actually deploy to CF successfully, if you can help trying that would help |
Drop the @takumi-rs/* v1 packages for takumi-js 2.0.0-beta.10. and remove the hacky wasm import plugin
Follow up on #893 (comment)
Summary by CodeRabbit