Proposal: To be able to easily run @valibot/i18n in Playground as well#1488
Proposal: To be able to easily run @valibot/i18n in Playground as well#1488ysknsid25 wants to merge 8 commits into
Conversation
|
@ysknsid25 is attempting to deploy a commit to the Open Circle Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR enables i18n language translations to function within the Valibot playground. It adds a build script that emits ESM bundles for each non-default locale, registers 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
ysknsid25
force-pushed
the
feat/playground
branch
from
daf12d3 to
7bd0d39
Compare
Signed-off-by: ysknsid25 <kengo071225@gmail.com>
ysknsid25
force-pushed
the
feat/playground
branch
from
7bd0d39 to
610942d
Compare
dosubot
Bot
added
size:L
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds @valibot/i18n support to the website playground by generating and serving per-language playground bundles and wiring them into the iframe import map (including local-dev and Vercel build adjustments).
Changes:
- Generate self-contained per-language i18n ESM bundles for the playground and include them in Vercel builds.
- Extend the playground iframe import map to expose
@valibot/i18n/<lang>URLs and add Monaco editor typings for i18n imports. - Adjust local dev server headers and documentation to support loading cross-origin modules in the sandboxed iframe.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| website/vite.config.ts | Adds dev-server CORS header to allow sandboxed iframe module loading locally. |
| website/vercel.json | Builds i18n playground bundles during deploy and adds long-term caching for emitted .mjs assets. |
| website/src/routes/playground/index.tsx | Imports i18n bundle URLs and serializes them into the iframe import map. |
| website/src/components/CodeEditor.tsx | Adds Monaco TypeScript module declarations for @valibot/i18n imports. |
| website/package.json | Adds @valibot/i18n workspace dependency. |
| website/README.md | Documents required i18n playground build step before starting the website. |
| packages/i18n/scripts/build-playground.ts | New script emitting one ESM bundle per language for the playground. |
| packages/i18n/package.json | Adds build.playground script entry. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
website/src/routes/playground/index.tsx (1)
36-67: 🏗️ Heavy liftAvoid maintaining locale lists in two places; generate/import from a single manifest.
The locale catalog is duplicated here and in
packages/i18n/scripts/build-playground.ts, which increases drift risk (missing import-map entries or dead bundles). Consider generating a manifest during i18n build and consuming it here.Also applies to: 72-111
🤖 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 `@website/src/routes/playground/index.tsx` around lines 36 - 67, The current hardcoded locale imports (e.g., i18nAr, i18nZhCN, etc.) duplicate the locale list maintained in packages/i18n/scripts/build-playground.ts, causing drift; instead generate a single manifest during the i18n build and consume it here: update build-playground.ts to emit a JSON or JS manifest of available playground locales (paths and keys) and replace the static imports in website/src/routes/playground/index.tsx with a dynamic import loop that reads that manifest and imports each entry (or uses import(url) at runtime), ensuring the unique symbols like the imported locale identifiers (i18nAr, i18nDe, i18nZh-TW) are derived from the manifest so the list is maintained in one place.
🤖 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.
Inline comments:
In `@packages/i18n/scripts/build-playground.ts`:
- Around line 3-34: Imports in build-playground.ts use extensionless local ESM
paths (e.g., ar, az, zhCN, zhTW) which violates the rule requiring .ts
extensions; update every local import statement (imports named ar, az, ca, cs,
de, el, es, fa, fi, fr, hu, id, it, ja, ko, kr, mn, nb, nl, pl, pt, ro, ru, sk,
sl, sv, tr, uk, uz, vi, zhCN, zhTW) to include the .ts file extension (e.g.,
import ar from '../src/ar.ts') so the module loader and ESLint rule are
satisfied.
In `@website/vercel.json`:
- Around line 89-97: The Vercel route rule for "/assets/(.*).mjs" only sets
Cache-Control, causing CORS failures for module fetches; update the headers
array for the "/assets/(.*).mjs" rule in website/vercel.json to include an
"Access-Control-Allow-Origin" header (use the same value used for the
"/assets/(.*)-index.min.mjs" rule, e.g., "*") alongside the existing
Cache-Control entry so .mjs module requests are allowed cross-origin.
---
Nitpick comments:
In `@website/src/routes/playground/index.tsx`:
- Around line 36-67: The current hardcoded locale imports (e.g., i18nAr,
i18nZhCN, etc.) duplicate the locale list maintained in
packages/i18n/scripts/build-playground.ts, causing drift; instead generate a
single manifest during the i18n build and consume it here: update
build-playground.ts to emit a JSON or JS manifest of available playground
locales (paths and keys) and replace the static imports in
website/src/routes/playground/index.tsx with a dynamic import loop that reads
that manifest and imports each entry (or uses import(url) at runtime), ensuring
the unique symbols like the imported locale identifiers (i18nAr, i18nDe,
i18nZh-TW) are derived from the manifest so the list is maintained in one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 31d1fdf8-edaa-452d-93aa-3d1f2d52a27c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
packages/i18n/package.jsonpackages/i18n/scripts/build-playground.tswebsite/README.mdwebsite/package.jsonwebsite/src/components/CodeEditor.tsxwebsite/src/routes/playground/index.tsxwebsite/vercel.jsonwebsite/vite.config.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 9 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…bing Signed-off-by: ysknsid25 <kengo071225@gmail.com>
Signed-off-by: ysknsid25 <kengo071225@gmail.com>
Signed-off-by: ysknsid25 <kengo071225@gmail.com>
|
Deployment failed with the following error: Learn More: https://vercel.com/docs/concepts/projects/project-configuration |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Signed-off-by: ysknsid25 <kengo071225@gmail.com>
…nd update playground routing Signed-off-by: ysknsid25 <kengo071225@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
website/src/routes/playground/index.tsx (2)
374-378: ⚡ Quick winPrefer
interfaceovertypefor object shapes.Per coding guidelines, object shapes should use
interfaceinstead oftypein TypeScript files.♻️ Apply guideline
-type EditorButtonsProps = { +interface EditorButtonsProps { class: string; model: Signal<NoSerialize<monaco.editor.ITextModel>>; executeCode$: QRL<() => void>; -}; +}As per coding guidelines: "Prefer
interfaceovertypefor defining object shapes in TypeScript"🤖 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 `@website/src/routes/playground/index.tsx` around lines 374 - 378, Replace the type alias EditorButtonsProps with an interface declaration to follow the coding guideline: change `type EditorButtonsProps = { ... }` to `interface EditorButtonsProps { class: string; model: Signal<NoSerialize<monaco.editor.ITextModel>>; executeCode$: QRL<() => void>; }`, preserving the exact property names and types (including `class`, `model` and `executeCode$`) and leaving other usage sites unchanged.
89-94: ⚡ Quick winPrefer
interfaceovertypefor object shapes.Per coding guidelines, object shapes should use
interfaceinstead oftypein TypeScript files.♻️ Apply guideline
-type LogLevel = 'log' | 'info' | 'debug' | 'warn' | 'error'; +type LogLevel = 'log' | 'info' | 'debug' | 'warn' | 'error'; -type MessageEventData = { +interface MessageEventData { type: 'log'; log: [LogLevel, string]; -}; +}As per coding guidelines: "Prefer
interfaceovertypefor defining object shapes in TypeScript"🤖 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 `@website/src/routes/playground/index.tsx` around lines 89 - 94, Replace the object-shape type alias MessageEventData with an interface: change "type MessageEventData = { type: 'log'; log: [LogLevel, string]; }" to an equivalent "interface MessageEventData { type: 'log'; log: [LogLevel, string]; }" while leaving the LogLevel union alias as-is; update any imports/uses that expect MessageEventData if needed to reference the interface name.
🤖 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.
Inline comments:
In `@website/src/routes/playground/index.tsx`:
- Line 69: The current mapping uses path.match(/\/src\/(.+)\.ts$/)![1] which
assumes the regex always matches and can crash if it doesn't; modify the
transformation to handle a null match: call path.match(...) into a local
variable (e.g., matchResult), check if matchResult is truthy, and only extract
matchResult[1] when present—either filter out non-matching paths before mapping
or return a safe fallback/throw a controlled error; update the code at the map
call site (the arrow function using path.match) to perform this null check
instead of using the non-null assertion.
---
Nitpick comments:
In `@website/src/routes/playground/index.tsx`:
- Around line 374-378: Replace the type alias EditorButtonsProps with an
interface declaration to follow the coding guideline: change `type
EditorButtonsProps = { ... }` to `interface EditorButtonsProps { class: string;
model: Signal<NoSerialize<monaco.editor.ITextModel>>; executeCode$: QRL<() =>
void>; }`, preserving the exact property names and types (including `class`,
`model` and `executeCode$`) and leaving other usage sites unchanged.
- Around line 89-94: Replace the object-shape type alias MessageEventData with
an interface: change "type MessageEventData = { type: 'log'; log: [LogLevel,
string]; }" to an equivalent "interface MessageEventData { type: 'log'; log:
[LogLevel, string]; }" while leaving the LogLevel union alias as-is; update any
imports/uses that expect MessageEventData if needed to reference the interface
name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52c7372f-5ff9-4cdd-b43c-79abc65e2532
📒 Files selected for processing (4)
packages/i18n/scripts/build-playground.tswebsite/.gitignorewebsite/src/routes/playground/index.tsxwebsite/vercel.json
✅ Files skipped from review due to trivial changes (1)
- website/.gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- website/vercel.json
…s from unexpected glob results Signed-off-by: ysknsid25 <kengo071225@gmail.com>
|
Thank you for this PR! How big is the |
|
Thank you for feedback, @fabian-hiller
This is each index.mjs file size.
As you understand, Each
Based on the following points, I believe we should only load the languages that users actually import. The fundamental idea is that this is a 'Playground,' and it's best to start minimally.
Based on the above, I believe we should only load the languages that users actually import. |
resolves: #1487
2026-06-03.0.38.40.mov
Summary by cubic
Make
@valibot/i18nwork in the Playground by shipping self-contained per-language ESM bundles and wiring them into the iframe import map. Users can import translations like@valibot/i18n/dein snippets with type safety.New Features
build.playgroundin@valibot/i18nto emit one self-contained ESM bundle per language; auto-discovers languages fromsrc(skipsen/types) and keepsvalibotexternal.website/publicand map them in the iframe import map as@valibot/i18n/<lang>; derive languages via globbing, add defensive mapping to ignore unexpected files, and avoid?urlto prevent Vite rewrites/double-loadingvalibot.@valibot/i18nto thewebsiteworkspace and declare@valibot/i18nmodules in Monaco for type safety..mjsassets (including/playground/i18n/*), and update Vercel build to run the i18n playground build and use the websitebuild.vercelscript.Migration
pnpm -F @valibot/i18n build.playground(seewebsite/README.md).Written for commit 105a81a. Summary will update on new commits.