i18n: Make sprintf return FormattedText for type-safe createInterpolateElement#76974
i18n: Make sprintf return FormattedText for type-safe createInterpolateElement#76974manzoorwanijk merged 6 commits intotrunkfrom
Conversation
Change sprintf's return type from `string` to `FormattedText<T>`, a
branded type that preserves the format string literal at the type
level. Update `InterpolationInput` and `InterpolationString` in
`@wordpress/element` to unwrap `FormattedText`, enabling tag
inference in `createInterpolateElement` when used with sprintf.
This makes the common pattern type-safe:
createInterpolateElement(
sprintf( '<Name>%s</Name>', name ),
{ Name: <span /> }
);
Closes #76972
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Flaky tests detected in 4ff1080. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24080749308
|
ConversionMap stays strict for all inputs — typos and unknown tags are caught. The 2 call sites that inject tags via sprintf arguments (an inherently type-unsafe pattern) get targeted @ts-expect-error suppressions with explanatory comments.
|
Size Change: +2 B (0%) Total Size: 7.74 MB 📦 View Changed
ℹ️ View Unchanged
|
| ), | ||
| { | ||
| div: <div aria-hidden />, | ||
| // @ts-expect-error — Tag injected via sprintf argument, not visible in format string. |
There was a problem hiding this comment.
What this code is doing is very weird, and the ts-expect-error is a warning flag that draws attention to it.
It obviously tries to do accessibility and good support for screen readers, but is it doing it well? There is the <CurrentPage /> component that shows a SelectControl whose value is the page number and the aria-label is "Current page". Then the <CurrentPage /> is wrapped by aria-hidden divs so that the whole thing reads as "Page X of Y" for screen readers, but shows only "X" to visual users.
@ciampo @mirka is there a better way to make such pattern (pagination) to do a11y better? We do pagination at many places around WordPress and Gutenberg, there is certainly a good existing way to display it.
There was a problem hiding this comment.
It does look kind of weird, but it's the result of a lot of back and forth to get the design, accessibility, and internationalization concerns right (mainly discussed in #63815). So while there probably isn't a quick improvement over the current, if anything we may want to componentize it, given this is copied from the one in DataViews.
Replace the separate FormattedText type with TranslatableText — they serve the same purpose (carrying a string literal through a branded type). sprintf now returns TranslatableText<T> directly. Also remove the unnecessary `as` casts on sprintf arguments, keeping only the return value cast.
Unify the branded string type under a single, neutral name since it is now used by both translation functions and sprintf. The old TranslatableText name is kept as a deprecated alias for backwards compatibility.
jsnajdr
left a comment
There was a problem hiding this comment.
Thank you, the new TransformedText type is great, with the legacy TranslatableText fallback.
CI complains about the optional dataview changelog, but given how trivial this PR's change is (one ts-expect-error comment), we can skip it.
…teElement (#76974) Co-authored-by: manzoorwanijk <manzoorwanijk@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org> Co-authored-by: mirka <0mirka00@git.wordpress.org>
What?
Changes
sprintf's return type fromstringtoTransformedText<T>, preserving the format string literal at the type level. UpdatescreateInterpolateElementto unwrap this type for tag inference.Also renames the existing
TranslatableText<T>branded type to the more neutralTransformedText<T>, since it's now shared by both translation functions andsprintf.TranslatableTextis kept as a deprecated alias for backwards compatibility.Why?
After #71513,
createInterpolateElementcan infer tag names from string literals. However, many call sites wrap the string insprintf:Since
sprintfreturnedstring, the literal type was lost and no tags were inferred. This was identified as a follow-up in #71513 (review).How?
TransformedText<T>in@wordpress/i18n— a phantom-brandedstring & { readonly __transformedText: T }that preserves the string literal type without any runtime cost. Replaces the oldTranslatableText<T>(kept as a deprecated alias).sprintfreturn type changed fromstring→TransformedText<T>. Since it extendsstring, this is backwards-compatible.InterpolationStringin@wordpress/elementupdated to unwrapTransformedText, extracting the format string for tag inference.ConversionMapstays strict — typos and unknown tags are caught at compile time.sprintf(_x('<div>Page</div>%1$s...'), '<CurrentPage />', ...)) get targeted@ts-expect-errorsuppressions. This pattern is inherently type-unsafe since tags in sprintf arguments aren't visible in the format string.Testing Instructions
Type-level — the compiler now catches invalid tags:
Runtime behavior is unchanged —
TransformedTextis a phantom type with no runtime footprint.Closes #76972