Assemble catalogues in the bundler module pipeline so edits hot-reload - #78
Assemble catalogues in the bundler module pipeline so edits hot-reload#78k0d13 wants to merge 8 commits into
Conversation
🦋 Changeset detectedLatest commit: 25ee5b7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe PR adds inline and module catalogue modes to the Babel plugin. It adds Next.js, Turbopack, and Metro integrations, shared catalogue loading, cache invalidation, examples, documentation, package exports, and release changesets. ChangesCatalogue loading and Babel boundary
Next.js and Turbopack integration
Metro transformer integration
Babel example and integration documentation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
packages/plugin-babel/src/metro/index.ts (1)
39-44: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueGuard against a double wrap.
If a caller applies
withSayKittwice,metroConfig.transformerPathalready points at this package'stransformer.cjs.saykitTransformerPaththen points at the SayKit transformer itself.upstream()inpackages/plugin-babel/src/metro/transformer.tsline 26 loads that same module, so every transform recurses until the stack overflows. Add a check that returns the config unchanged when it is already wrapped.🛡️ Proposed guard
+ const self = join(__dirname, 'transformer.cjs'); + if (metroConfig.transformerPath === self) return metroConfig; + const upstream = metroConfig.transformerPath ?? 'metro-transform-worker';🤖 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 `@packages/plugin-babel/src/metro/index.ts` around lines 39 - 44, Update the wrapper logic around the transformerPath assignment in withSayKit to detect when metroConfig.transformerPath already resolves to this package's transformer.cjs and return metroConfig unchanged. Ensure the existing wrapping behavior remains intact for unwrapped configurations and prevents transformer.ts upstream() from loading the SayKit transformer recursively.packages/plugin-babel/src/index.test.ts (1)
79-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the fallback test self-contained.
This test asserts
farewell: 'Bye'. That key only exists because the previous test at lines 66-77 wroteen.jsoninto the shareddir. The test then depends on file order and on the earlier test running. Writeen.jsoninside this test as well, or move the shared fixture into abeforeEach.🤖 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 `@packages/plugin-babel/src/index.test.ts` around lines 79 - 90, The fallback test around loadCatalogue must create its own source-locale fixture instead of relying on the preceding test's en.json. Write the required en.json data within the test, or initialize the shared fixture in beforeEach, while preserving the existing assertions for greeting, farewell, and sources.
🤖 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 @.changeset/hot-donkeys-shave.md:
- Line 5: Update the release-note wording around the Metro integration to
qualify the hot-reload claim: state that active-locale catalogue edits
hot-reload, while fallback-locale edits may remain stale because Metro does not
register fallback files as dependencies.
- Line 2: Update the one-line release note for babel-plugin-saykit to include
the migration requirement: existing consumers must add the
babel-plugin-saykit/webpack loader or babel-plugin-saykit/metro configuration so
catalogue imports are transformed instead of remaining in source form.
In `@packages/plugin-babel/src/metro/index.ts`:
- Around line 36-49: Update the upstream resolution in the Metro configuration
flow to derive the createRequire base from the caller-provided config path
rather than process.cwd(), so --config paths and monorepos resolve the project’s
Metro dependency correctly. Preserve absolute upstream paths, and fall back to
this module’s own resolver when no usable config path is available.
In `@packages/plugin-babel/src/metro/transformer.ts`:
- Around line 55-57: Update getCacheKey to combine the upstream transformer key
with a SayKit-specific salt derived from this package’s version and stable
SayKit configuration fields, including fallback-chain and bucket-formatter
settings. Do not serialize the entire config when it may contain functions; use
a deterministic representation of only serializable stable fields so
configuration changes invalidate Metro’s cache.
In `@website/content/integrations/babel.mdx`:
- Around line 33-49: Update the Next.js configuration example around the
catalogue and webpack rules to document JSON catalogue handling: add a matching
*.json Turbopack rule and webpack loader rule, or explicitly state that JSON
catalogues require their own rule, while preserving the existing *.po
configuration.
---
Nitpick comments:
In `@packages/plugin-babel/src/index.test.ts`:
- Around line 79-90: The fallback test around loadCatalogue must create its own
source-locale fixture instead of relying on the preceding test's en.json. Write
the required en.json data within the test, or initialize the shared fixture in
beforeEach, while preserving the existing assertions for greeting, farewell, and
sources.
In `@packages/plugin-babel/src/metro/index.ts`:
- Around line 39-44: Update the wrapper logic around the transformerPath
assignment in withSayKit to detect when metroConfig.transformerPath already
resolves to this package's transformer.cjs and return metroConfig unchanged.
Ensure the existing wrapping behavior remains intact for unwrapped
configurations and prevents transformer.ts upstream() from loading the SayKit
transformer recursively.
🪄 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 Plus
Run ID: 1d0db4ad-34f5-4afc-b573-7df751f9935c
📒 Files selected for processing (12)
.changeset/hot-donkeys-shave.mdexamples/expo/metro.config.jsexamples/nextjs/next.config.mjspackages/plugin-babel/package.jsonpackages/plugin-babel/src/catalogue.tspackages/plugin-babel/src/index.test.tspackages/plugin-babel/src/index.tspackages/plugin-babel/src/metro/index.tspackages/plugin-babel/src/metro/transformer.tspackages/plugin-babel/src/webpack/index.tspackages/plugin-babel/tsdown.config.tswebsite/content/integrations/babel.mdx
5ab74f9 to
8692d4d
Compare
@saykit/config
@saykit/format-json
@saykit/format-po
saykit
@saykit/carbon
@saykit/react
babel-plugin-saykit
unplugin-saykit
@saykit/transform-js
@saykit/transform-jsx
commit: |
|
The preview build of |
a109168 to
2a77e73
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@examples/babel/README.md`:
- Line 17: Update the fenced output block in the README to specify the text
language, changing the opening fence to use the text identifier so it satisfies
markdownlint MD040.
- Around line 3-4: Update the build command in the Babel example README to use
the package.json build script, `pnpm build`, instead of the incomplete direct
Babel invocation; keep the subsequent `node dist/main.js` run command unchanged.
In `@packages/plugin-babel/src/index.ts`:
- Around line 49-65: Update the import handling around the specifier lookup to
require exactly one ImportDefaultSpecifier and reject declarations containing
any named or namespace specifiers, preserving the existing code-frame error
behavior. Add a regression test covering `import messages, { extra } from
'./messages.json'` and verify the mixed import is rejected rather than removing
the additional binding.
In `@packages/plugin-babel/src/metro/index.ts`:
- Line 52: Update resolveConfigFile and resolveConfig to accept an optional
project-directory parameter and use it for config discovery instead of always
relying on process.cwd(). In the Metro wrapper around the config resolution
call, pass metroConfig.projectRoot ?? process.cwd(), preserving the existing
fallback and ensuring the same root used by the upstream transformer is applied.
In `@packages/plugin-babel/src/metro/transformer.ts`:
- Line 27: Update the transformer’s resolve flow around config and cache
initialization to pass Metro’s projectRoot into resolveConfig and
resolveConfigFile, replacing implicit process.cwd() lookup. Key all
resolved-config results and cache-salt values by that same projectRoot so
separate projects cannot share configuration or caches.
In `@website/content/integrations/babel.mdx`:
- Around line 99-103: Update the Babel integration description around the
catalogue import paragraph to distinguish catalogue modes: explain that Babel
resolves and merges catalogue files only for inline mode, while module mode
preserves the import for the Next.js loader or Metro transformer to assemble.
Keep the surrounding transformer and runtime behavior description accurate.
🪄 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 Plus
Run ID: f1789780-20e3-4a38-aee0-9f84ee601db1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
.changeset/fresh-owls-guess.md.changeset/hot-donkeys-shave.md.changeset/olive-pugs-repeat.md.changeset/tidy-moons-agree.md.vscode/settings.jsonexamples/babel/README.mdexamples/babel/babel.config.jsexamples/babel/package.jsonexamples/babel/saykit.config.tsexamples/babel/src/locales/en.d.po.tsexamples/babel/src/locales/en.poexamples/babel/src/locales/fr.d.po.tsexamples/babel/src/locales/fr.poexamples/babel/src/main.tsexamples/babel/tsconfig.jsonexamples/expo/babel.config.jsexamples/expo/metro.config.jsexamples/nextjs/.babelrcexamples/nextjs/next.config.mjspackages/config/src/features/loader/index.tspackages/config/src/features/loader/resolve.tspackages/plugin-babel/package.jsonpackages/plugin-babel/src/catalogue.tspackages/plugin-babel/src/index.test.tspackages/plugin-babel/src/index.tspackages/plugin-babel/src/metro/index.tspackages/plugin-babel/src/metro/transformer.tspackages/plugin-babel/src/next/index.tspackages/plugin-babel/src/next/loader.tspackages/plugin-babel/tsdown.config.tswebsite/content/integrations/babel.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
- .changeset/hot-donkeys-shave.md
- packages/plugin-babel/tsdown.config.ts
- examples/expo/metro.config.js
Fixes #71.
Catalogues were inlined into whoever imported them, which bakes the record into a module whose own bytes never change. No bundler can invalidate on that, so editing a catalogue did nothing until a cache-clearing restart (
expo start --clear). Next.js was affected identically.addExternalDependencycould never have helped: Babel freezes the array the moment the plugin factory returns (config/helpers/deep-array.js→Object.freeze), so calling it from a visitor always threw into thecatch {}. And neither Metro nor Next consumesexternalDependenciesanyway — Next declares it in its loader's config type and never turns it into a webpack dependency. Full analysis in the issue.Approach
Catalogues stay real modules, assembled by each bundler's own module pipeline — the only layer with an invalidation signal.
Metro needs a transform worker rather than a loader because it routes
.jsonstraight throughtransformJSONand never runs Babel over it.withSayKitalso registers non-JSON catalogue extensions with Metro's resolver, since a file Metro can't resolve isn't a module it can reload.These two are the whole list of integrations, because they are the two bundlers not reachable any other way. On webpack, Vite, Rollup or esbuild proper,
unplugin-saykitalready does this through each bundler's own plugin API — there is nobabel-plugin-saykit/webpack, and the loader is published only as./next/loaderso Turbopack can name it in a rule.Assembly is opt-in
Inlining stays the default, so
babel-plugin-saykiton its own behaves exactly as before and this is not a breaking change.The two are mutually exclusive by nature: if the plugin inlines the import, the integration is never asked for the module. Hence an explicit option rather than a default flip.
Next.js setup is one line
withSayKitderives the rules fromsaykit.config.*rather than asking you to hand-write them:One rule per bucket, for Turbopack and for
next --webpack, each targeting that bucket'soutputexactly — a Turbopack glob built from theoutputtemplate, and a webpack predicate. That removes two footguns that were previously documentation warnings:.jsonrule would have swept up every JSON import in the app. Deriving fromoutputmeans only catalogues match. The generated rules also declare the loader's output as JavaScript (type: 'javascript/auto',as: '*.js'), which webpack does not assume for.json.Verified against running dev servers
fr.powith the server runningen.powhile viewing/fr(fallback-only dependency)addDependencyfr.jsonwith Metro running, no--clearen.jsonwhilefrfalls back to itnext build(Turbopack) prerenders/en,/fr,/plwith translations in place.next build --webpackfails in the example, but on@messageformat/parserbeing transpiled by Next's bundled Babel — anode_modulesfile no catalogue rule touches, and unrelated to this PR. So the webpack half ofwithSayKitis covered by unit tests (rule shape, and the predicate matching catalogues but not neighbouring source files) rather than a real build.429 tests,
pnpm check19/19, lint and format clean.Known gap (Metro only)
Metro keys its transform cache on each file's own bytes, and
getCacheKeytakes no filename — it is one global key — so there is no hook to hang a fallback chain's contents on. A fallback locale's contents baked into another locale's record can't be invalidated. Documented as a callout on the Babel integration page. Editing the locale you're viewing — the case in the issue — works.Tracked as #81, with the analysis: the fix is emitting
module.exports = Object.assign({}, require('./en.json'), {…})so the merge happens over real module edges. Two things make it its own PR —metro-transform-workerroutes.jsonintotransformJSON, which never scans for dependencies, and each module has to emit its translations only so a fallback'smsgidcan't outrank a real translation.Also in here
projectRootinstead ofprocess.cwd(), so--configand monorepo layouts resolve the project's own Metro. Falls back to this package's resolver for hoisted installs.withSayKitis now a no-op if it would wrap its own transformer, which would otherwise recurse until the worker died.saykit.config.*contents and this package's version. Previously, editing the fallback chain or a bucket formatter left every catalogue serving its cached record. The config path comes from the newresolveConfigFile, so it cannot silently disagree with the config that was actually loaded.examples/babel— Babel and nothing else,babel src --out-dir distthennode dist/main.js. Every other example runs the plugin alongside a bundler, so this is the only one that would notice if the inlining default broke.@saykit/configexportsresolveConfigFile.typespaths, which pointed at.d.cjswhile tsdown emits.d.cts— TS consumers were getting no types at all.