feat(core): Resolve configurable operation strings from i18n catalogs - #5079
feat(core): Resolve configurable operation strings from i18n catalogs#5079michaelbromley wants to merge 12 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Dashboard Preview: https://admin-dashboard-4wo69zgtl-vendure.vercel.app |
grolmus
left a comment
There was a problem hiding this comment.
Second-eyes review @michaelbromley — this is really cleanly built. The token-based CONFIGURABLE_OPERATION_TRANSLATOR (rather than importing I18nService into the low-level base class), reading via getResourceBundle + manual path walk instead of t() to preserve { arg } placeholders, and the ConfigDefType = ConfigurableOperationDefType alias with the compile-error-on-omission guarantee are all the right calls. The resolution order (per-language: catalog → inline, so a plugin's own non-English inline string isn't beaten by core's English catalog) is subtle and correctly handled, and the test suite genuinely covers it — including the shared-ui-mutation case with the follow-up assertion that the source array isn't truncated. Nothing blocking below; one thing worth confirming and a few minor notes.
Worth confirming: ui.options[].label shape narrowing
Previously toGraphQlType passed ui: arg.ui through verbatim, so ui.options[].label reached the client as the full multi-language LocalizedStringArray. localizeUiOptions now resolves it server-side to a single-entry array:
? option
: { ...option, label: [{ languageCode: ctx.languageCode, value: label }] };Two consequences:
- The stamped language can be inaccurate on fallback. The entry is always stamped with
ctx.languageCode, even when the resolved value came from the channel default or the English inline fallback — e.g. aderequest with only an English inline option label returns[{ languageCode: de, value: 'Automatic' }]. Harmless for display, but the array no longer tells the truth about what language the string is in. - Client-side language switching no longer re-localizes option labels. The dashboard consumer (
select-with-options.tsx) doeslabel.find(t => t.languageCode === displayLanguage). At fetch timelanguageCode === displayLanguage, so it matches and works today. But a cache-then-render-at-a-different-language flow would now fall through tolabel[0]instead of picking the right language from a full array. This is consistent with howdescriptionand arglabelalready resolve to a single string server-side, so it's a narrowing rather than a regression — just want to confirm no admin-ui/dashboard flow relies on receiving all languages for option labels in one fetch. (The mutation-safety e2e covers cross-request isolation well, but not this cache-then-switch angle.)
Minor
if (fromCatalog)inlocalizeStringis a truthiness check, so an intentionally empty-string catalog entry is treated as "no translation" and a catalog can't blank out an inline string. Almost certainly intended — just flagging it's truthiness, not!== undefined.- The PR ships the mechanism with no populated core catalogs (
src/i18n/messages/<lng>.jsonaren't in the diff), so every core op still resolves to its inline strings until the tooling is run and committed. Assuming that's the intended "plumbing now, content follow-up" split — worth a line in the description confirming it. - Doc nit: a few docblocks / the guide say
getTranslationKeys()is used to "print" the key paths — it returns them. - Test gap:
getTranslationKeysis covered for a normal op and the no-defTypecase, but not for an arg whoseuihas no options (non-select) —getUiOptionshandles it, but there's no direct assertion that such an arg contributes no option keys.
Thanks — nice mechanism, and the "no user-visible change, translations are a follow-up" framing makes it easy to reason about.
|



Configurable operation descriptions and arg labels are currently defined inline in each operation's
.tsfile as aLocalizedStringArray. Supporting a new language means editing every one of those files, which is why core has never gone beyond a handful of languages for these strings while the dashboard has reached 28.This adds a second source for those strings: the server-side i18n message catalogs core already ships. There is no user-visible change in this PR — the mechanism lands, and the 1242 translations it enables are a follow-up.
Approach
Resolution happens server-side in
toGraphQlType, so the GraphQL contract is unchanged and existing API consumers keep working.The keys are derived from the operation itself, so
ConfigurableOperationDefOptionsis untouched and every existing plugin works unmodified:<type>is the registry the operation belongs to. It has to be part of the key because a code is only unique within its own registry — core itself usesbuy_x_get_y_freefor both aPromotionConditionand aPromotionAction.English stays inline. The catalogs hold translations only, so a missing translation falls back to the English already in the source file rather than to a raw key, and adding a 29th language adds zero lines to any
.tsfile.Resolution order
For each language in turn — the requested language, then the Channel's default, then English — the catalog is checked first, then the inline array:
Checking both sources per language rather than exhausting the catalogs first matters: the other ordering would reach core's English catalog entry before a plugin's own German inline string, silently breaking every plugin that ships non-English strings today.
Catalog beating inline for the same language is deliberate, and is what allows a user to override a third-party operation's wording via
addTranslation()without forking it.Why the lookup bypasses
t()Six core descriptions are templates rather than sentences, such as
Discount order by { discount }%. The Admin UI substitutes the live argument values client-side, so the braces have to survive the round trip untouched. Core's i18next is initialised with.use(ICU), which would treat{ discount }as an argument reference.I18nService.getConfigurableOperationTranslation()therefore reads the resource straight out of the store and walks the path segment by segment. That also sidesteps i18next's key splitting, so a plugin is free to use a code containing dots such asacme.shipping.handler.Changes
ConfigurableOperationDef, covering the operation description, each arg's label and description, andui.options[].labelgetConfigurableOperationTranslation()onI18nService, plus aconfigurableOperationmember onVendureTranslationResources(optional, so existing typed resource objects still compile)Symboltoken incommon/constants.ts, registered byI18nModuleasuseExisting: I18nServiceand captured in the baseinit(injector). A token rather than the class itself keeps the low-levelConfigurableOperationDeffrom importing the i18n module.defTypeon the nineConfigurableOperationDefsubclasses.ConfigDefTypein the registry is now an alias of the newConfigurableOperationDefTypeunion, so indexingConfigDefTypeMapby it keeps the two lists in step.getTranslationKeys(), which returns each key path with its English source value. Plugin authors can call it to check a key; the extraction tooling uses it as its input.npm run i18n:extract/i18n:applyinpackages/core, modelled on the dashboard's tooling and sharing itslocale-profiles.jsscript validation, so a batch labelledruwith no Cyrillic in it is refused rather than written to disk.Behaviour changes
Two small ones, both improvements:
description: []array previously threw insidelocalizeString. It now resolves to an empty string.labelordescriptionis now resolved even when the operation defines no inline array for it, so a label can be supplied purely from a catalog.Testing
configurable-operation.spec.ts, including the case that proves existing plugins are not broken, and the case proving the shareduiconfig is not mutated across requeststranslations.e2e-spec.tsexercising the token-based DI wiring end to end, which is the part that would fail silently if the token were wrongtoGraphQlType: translations, promotion, order-promotion, promotion-side-effects, shipping-method, shipping-method-eligibility, collection, payment-method, payment-process, configurable-operation-secrets (235 passed)Follow-ups
configurableOperationtranslations for all 27 non-English locales.i18n:extractreports 46 distinct strings across 23 core operations, so 1242 translations. This is the point at which a Czech administrator finally sees Czech.error/errorResult/messagenamespaces from 8 locales to 28. They have already drifted —en.jsonhas 58errorkeys,de.jsonhas 45,ru.jsonhas 44.labelanddescription. The same underlying pain, but a different architecture:CustomField.labelgoes over the wire as a fullLocalizedString[]and is resolved client-side, so covering it means a breaking schema change.Relates to #4820
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.