-
-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: execute function in I18nProviderWrapper instead of I18nProvider #316
Conversation
Someone is attempting to deploy a commit to a Personal Account owned by @QuiiBz on Vercel. @QuiiBz first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much! Just tried and everything seems to be working as expected. Some suggestions:
packages/next-international/src/app/client/create-i18n-provider-client.tsx
Outdated
Show resolved
Hide resolved
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. Unfortunately adding back the artificial delay shows that we now have a loading state when switching locale in client components, which we don't want:
Screen.Recording.2023-12-23.at.08.57.47.mov
I believe we'll need to add back the localesCache
. use()
always triggers suspense so we should only use it when loading the page for the first time.
Based on your feedback, the I've also added a check in I think in the // locales/client.ts
export const { useI18n, useScopedI18n, I18nProviderClient, useChangeLocale, defineLocale, useCurrentLocale } =
createI18nClient(
{
en: async (isChanging: boolean = false) => { // <---- HERE
if (!isChanging) {
await new Promise(resolve => setTimeout(resolve, 100));
}
return import('./en');
},
fr: async (isChanging: boolean = false) => { // <---- HERE
if (!isChanging) {
await new Promise(resolve => setTimeout(resolve, 100));
}
return import('./fr');
},
},
{
// Uncomment to set base path
// basePath: '/base',
// Uncomment to use custom segment name
// segmentName: 'locale',
// Uncomment to set fallback locale
// fallbackLocale: en,
},
); and in the // packages/next-international/src/app/client/create-use-change-locale.ts
export function createUseChangeLocale<LocalesKeys>(
useCurrentLocale: () => LocalesKeys,
locales: ImportedLocales,
config: I18nClientConfig,
) {
return function useChangeLocale(changeLocaleConfig?: I18nChangeLocaleConfig) {
const { push, refresh } = useRouter();
const currentLocale = useCurrentLocale();
const path = usePathname();
// We call the hook conditionally to avoid always opting out of Static Rendering.
// eslint-disable-next-line react-hooks/rules-of-hooks
const searchParams = changeLocaleConfig?.preserveSearchParams ? useSearchParams().toString() : undefined;
const finalSearchParams = searchParams ? `?${searchParams}` : '';
let pathWithoutLocale = path;
if (config.basePath) {
pathWithoutLocale = pathWithoutLocale.replace(config.basePath, '');
}
if (pathWithoutLocale.startsWith(`/${currentLocale}/`)) {
pathWithoutLocale = pathWithoutLocale.replace(`/${currentLocale}/`, '/');
} else if (pathWithoutLocale === `/${currentLocale}`) {
pathWithoutLocale = '/';
}
return function changeLocale(newLocale: LocalesKeys) {
const importFnLocale = locales[newLocale as keyof typeof locales];
if (!importFnLocale) {
warn(`The locale '${newLocale}' is not supported.`);
return;
}
importFnLocale(true).then(module => { // <---- HERE
localesCache.set(newLocale as string, module.default);
push(`/${newLocale}${pathWithoutLocale}${finalSearchParams}`);
refresh();
});
};
};
} As we don't need to wait for 100ms when changing the locale from the client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There's no need for isChanging
for now, this is an implementation detail that shouldn't be exposed to the user. And keeping the timeout is very important in this example, to reveal issues like the one above in slow networks.
I'll publish a release in the next few days!
Closes #290
With this fix, I wasn't able to reproduce the crash.
What is this PR ?
It moves the call of
() => import("./en")
fromI18nProvider
toI18nProviderWrapper
.I changed this because according to the React documentation of
use
hook (https://react.dev/reference/react/use#streaming-data-from-server-to-client), the promise is executed and passed as a prop from the parent but awaited in the child.