feat(i18n): introduce new design for IntlProvider and useIntl - #5684
feat(i18n): introduce new design for IntlProvider and useIntl#5684ngolin wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
df270a1 to
be195ac
Compare
PR Analysis Report
New Componentsintl (@astryxdesign/core)
Modified ComponentsAlertDialog (@astryxdesign/core)
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Visual RegressionStatus: Skipped — Broad stable scope is deferred to the daily release gate. It covers 4332 trusted baseline shots instead of recapturing them for this PR. View the report Generated by PR Enrichment workflow | View full report |
nynexman4464
left a comment
There was a problem hiding this comment.
What do you mean by legacy API? Why is this introducing a new API? Please provide some context and rationale.
Hi @nynexman4464, sorry for that, the commit message was AI-generated and didn't capture the full context. I'll provide more details later for your reference. |
But why what is wrong with the current provider? I'm confused by what this PR is trying to achieve. Are you having issues with the current provider? |
Hi @nynexman4464, @cixzhang — there’s nothing wrong with the current provider; the existing implementation works. My goal with this change is to make the intl surface clearer and safer for both maintainers and consumers. Concretely I want to:
Design notes (high level):
interface ProviderValue<MyMessages> {
locale: string;
// `overrides` used to replace system component strings/catalog
overrides: Record<string, string>;
// `messages` are for the user's own components (not for system catalog)
messages?: MyMessages;
}
// for maintainers
const {direction, t} = useIntl();
t('@astryx.pagination.next'); // ok
t('@astryx.not.existed.key'); // error (caught at dev time)
// for consumers
const {messages, t} = useIntl<MyMessages>();
const {currency, discount} = messages; // fully type-safe
t('@astryx.pagination.next'); // use system messagesDemo examples
// Replace the system's default catalog
import zh from 'locales/zh-CN.json';
<Provider locale="zh" overrides={zh} />
// Replace catalog and tweak a few keys
<Provider locale="zh" overrides={{...zh, '@astryx.pagination.next': 'Next'}} />
interface MyMessages {
currency: React.ReactNode;
discount: (percent: number) => React.ReactNode;
}
const en: MyMessages = {
currency: <DollarIcon />,
discount: percent => `${100 - percent}% Off`,
};
const zh: MyMessages = {
currency: <ChinaYuan />,
discount: percent => `${percent / 10} 折`,
};
const messages = {en, zh};
const locale = 'zh';
<IntlProvider locale={locale} messages={messages[locale]}>
<MyComponent price={10} percent={30} />
</IntlProvider>;
const MyComponent = ({price, percent}: {price: number; percent: number}) => {
const {messages} = useIntl<MyMessages>();
const {currency, discount} = messages;
return (
<span>
{price}
{currency}, {discount(percent)}
</span>
);
}; |
- Add a dedicated new design for the intl subpackage with IntlProvider, IntlContext and useIntl - Demonstrate how to migrate from useTranslator to useIntl using AlertDialog - Show how to achieve compatibility between InternationalizationContext and IntlProvider via getIntlContextValue
|
I'm still confused by this proposal. It sounds like it follows the principles we set out to design our internationalization system (RFC #3641). It's intended to be used internally, but could be used as the internationalization provider for any app, as outlined in the docs. Granted it's not ideal for that case, but we're not trying to build a fully featured internationalization system. There are lots of good-quality existing systems like react-intl, next-intl, react-i18next, and Lingui that are already more full featured.
I don't know if allowing arbitrary react components is something we want. I do see other i18n systems have a "rich text" mode for converting XML like markup to some kind of component, but currently we don't have the need for this in astryx. It adds a lot of complexity for not a lot of benefit at this time. I appreciate the proposal here, but I'm not sure this is right for astryx at this time. |
Uh oh!
There was an error while loading. Please reload this page.