Repository navigation
P7: compile messages to data - #110
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 5f70e0c 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.
|
github-actions
Bot
added
dependencies
Contributor
|
Important Review skippedToo many files! This PR contains 175 files, which is 75 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.
 |
k0d13
force-pushed
the
compile-messages
branch
from
September 21, 2026 11:12
a7669f4 to
2f0b28d
Compare
github-actions
Bot
added
package: transform-js
Owner
Author
|
@greptileai review @coderabbitai review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Contributor
|
|
k0d13
added
the
preview
@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: |
Contributor
|
The preview build of |
github-actions
Bot
removed
the
preview
k0d13
force-pushed
the
compile-messages
branch
from
October 17, 2026 01:01
2f0b28d to
3a91803
Compare
Owner
Author
|
@greptile review |
k0d13
added
the
preview
github-actions
Bot
removed
the
preview
k0d13
force-pushed
the
compile-messages
branch
from
October 17, 2026 03:25
cdb206f to
9b7a70f
Compare
Owner
Author
|
@kevinatbuape what are your thoughts on this pr, and how it might work with kiai |
|
@k0d13 Based on the design described here, moving ICU parsing to build time looks like a good fit for Kiai: a smaller dashboard runtime, with the same compiled catalogues potentially reusable for bot responses. Serializable messages also make server-to-client locale handoff cleaner. I’d keep locale/view caches request-scoped on the server so concurrent users never share the wrong locale, and measure real catalogue size plus cold/steady-state formatting before calling it a performance win. For adoption, I’d trial one dashboard flow and one bot response, checking plural/select behaviour, fallback locales, date/number output and Discord mention/Markdown rendering with bidi isolation. This is an architectural take from the PR description, not a code review or tested integration. Request my review through GitHub if you want the full PR review workflow. |
… and drop duration
k0d13
force-pushed
the
compile-messages
branch
from
October 28, 2026 04:17
60d9d83 to
8b68bfd
Compare
k0d13
added
the
preview
github-actions
Bot
removed
the
preview
k0d13
added
the
preview
github-actions
Bot
removed
the
preview
Owner
Author
|
@greptile review |
Comment on lines
+113
to
+120
| if (this.bucket.match(event.filename)) { | ||
| await this.update(path); | ||
| continue; | ||
| } | ||
|
|
||
| // A translation changed, not a call site: recompile only, since extraction writes just the | ||
| // source locale | ||
| if (this.bucket.output.match(event.filename)) await this.compile(); |
There was a problem hiding this comment.
If a catalogue also matches the bucket's include glob, such as include: ['src/**/*'] with JSON catalogues under src/locales, this branch calls update() and skips the catalogue check. The transformer finds no messages in the JSON, so nothing recompiles and the app keeps showing old translations. Check catalogue paths before source paths.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/config/src/features/workers/extract-worker.ts
Line: 113-120
Comment:
**Watch mode misses translation edits**
If a catalogue also matches the bucket's `include` glob, such as `include: ['src/**/*']` with JSON catalogues under `src/locales`, this branch calls `update()` and skips the catalogue check. The transformer finds no messages in the JSON, so nothing recompiles and the app keeps showing old translations. Check catalogue paths before source paths.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| a message's values in their own namespace, so a value named `id` cannot displace the message being | ||
| looked up. `call` strips exactly one underscore back off, and passes keys written without one | ||
| through untouched, so a hand-written or custom-transformer-generated call works either way. | ||
| The descriptor names a message by `id`, or carries a compiled `message` itself, as for a lone `say.date(x)`. The other keys are values: `call` strips one leading underscore from each, and passes a key without one through as is. Throws if the id is not in the view. |
There was a problem hiding this comment.
The call() reference still says it strips underscores and accepts keys without them. The new runtime passes the descriptor unchanged, and compiled messages read exact keys such as _name. Following this advice with call({ id, name: 'Ada' }) leaves the placeholder empty. Document that callers must use the keys stored in the compiled message.
Suggested change
| The descriptor names a message by `id`, or carries a compiled `message` itself, as for a lone `say.date(x)`. The other keys are values: `call` strips one leading underscore from each, and passes a key without one through as is. Throws if the id is not in the view. | |
| The descriptor names a message by `id`, or carries a compiled `message` itself, as for a lone `say.date(x)`. The other keys supply values using the exact keys stored in the compiled message. ICU arguments compile to underscore-prefixed keys, so `{name}` reads `_name`; `call` does not strip underscores. Throws if the id is not in the view. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: website/content/reference/api/saykit.mdx
Line: 130
Comment:
**Reference promises unsupported value keys**
The `call()` reference still says it strips underscores and accepts keys without them. The new runtime passes the descriptor unchanged, and compiled messages read exact keys such as `_name`. Following this advice with `call({ id, name: 'Ada' })` leaves the placeholder empty. Document that callers must use the keys stored in the compiled message.
```suggestion
The descriptor names a message by `id`, or carries a compiled `message` itself, as for a lone `say.date(x)`. The other keys supply values using the exact keys stored in the compiled message. ICU arguments compile to underscore-prefixed keys, so `{name}` reads `_name`; `call` does not strip underscores. Throws if the id is not in the view.
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.…clude, and document that call strips no underscores
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #109, closes #81.
The plugin emitted ICU strings and the runtime parsed and formatted them on demand, so the client got
messageformat, its parser, both skeleton parsers, and a format cache whose only job was to avoid paying for the same parse twice. Everything about a message is known at build time, so none of that has to ship.saykit compilenow writes one module per locale, and that module is data:A view compiles a node into a closure the first time that message is called, and memoises it. What ships is a small evaluator over
Intl, and nothing parses anything.What changed
@saykit/configcompiles ICU to aMessagetree instead of to function source. Styles still resolve at build time, into theIntloptions bag they stand for, and skeletons and literal patterns are supported as beforeKind, a number exported fromsaykit(Value,Number,DateTime,Plural,Ordinal,Select,Hash), and any other array is a concatenation. Operands are descriptor keys rather than nested nodes, so{n, number}is[1, "_n"]. The numbers are the format: a kind is only ever appended, never renumbered#is its own node rather than a copy of the value. An ordinal is its own kind rather than a plural with a flagsaykitgainsMessageandcompileMessage(message, locale, options).View.Messagesis a record of nodes, andview.call()compiles on first use and caches per viewIntlobjects are built once per message, not looked up per call. A value that cannot be formatted, such as an invalid date, is written as given with a warning instead of throwingbidiIsolationcarries over from Add bidiIsolation option to createCatalogue and createView #115: written values are isolated the way MessageFormat 2 does by default, and'none'turns it offSayProvideris back tostore, orlocaleandmessages(plusoptions), and<SayProvider>on the server takes no props again — thereact-serverbuild reads the established view and hands everything acrosssaykit/runtimeis no longer a public entry point. Nothing imports it now that generated modules are data, so it is bundled into the main entrydurationargument type is dropped. No macro could author it, andIntlhas nothing to write it with, so a hand-written{s, duration}now writes the bare value, asspelloutdoesStale fallbacks in Metro (#81)
Metro's cache served a stale fallback because the Metro transformer assembled
fr.json's fallback chain inside Metro, where editingen.jsonleftfr.json's bytes, and so its cache entry, untouched. That transformer is gone. The CLI merges each locale's fallback chain into its own{locale}.js, andsaykit extract --watchrecompiles every locale when any catalogue changes, so editingen.jsonrewritesfr.jsand Metro reloads it like any other changed module. Without the watcher,saykit compiledoes the same.Why an AST, and not one function per message
#95 proposed emitting the function directly:
That was the first pass of this branch. It is smaller and faster, and it has one problem with no workaround worth having: a function cannot cross the RSC boundary.
With compiled functions a server component cannot hand its messages to a client tree at all. The app has to import the catalogue in a client module and pass only the locale, which means a
providers.tsxper app, a catalogue reachable from a module that is notserver-only, and every locale imported eagerly on the client so hydration matches. That branch existed, the nextjs example carried all of it, and it is a real tax for something the compiler is supposed to remove.Data crosses. So the provider goes back to taking
localeandmessages,<SayProvider>on the server takes no props, and no example needs a providers module.Two things fall out of it that are worth having on their own:
The cost is one indirection per node at format time, against a template literal the engine JITs, after a compile that happens once per message per locale and only for messages that render. A message with no placeholders is emitted as a bare string and skips all of it.
Examples
nextjs:<SayProvider>with no props inside thewithSay-wrapped layout,getSay()forlang. No providers moduletanstack-start: unchanged,localeandmessagesfrom the loader is still the right form.d.tsis nowRecord<string, Message>