chore(bundle): don't send translations to client #8447
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website @nodejs/web-infra Please review the changes when you have a chance. Thank you! 🙏 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8447 +/- ##
==========================================
+ Coverage 73.65% 73.73% +0.08%
==========================================
Files 108 108
Lines 9193 9200 +7
Branches 312 312
==========================================
+ Hits 6771 6784 +13
+ Misses 2420 2414 -6
Partials 2 2 ☔ View full report in Codecov by Sentry. |
📦 Build Size ComparisonSummary
Changes➕ Added Assets (10)
➖ Removed Assets (24)
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the i18n package to prevent locale translations from being bundled to the client. The main change removes the importLocale function and converts exported functions (getters) into direct constant exports, while updating all consuming code to import directly from the i18n package instead of through the wrapper file.
Key changes:
- Removed
importLocalefunction and converted getter functions to constant exports in the i18n package - Deleted the
apps/site/next.locales.mjswrapper file - Updated all import statements across the codebase to import directly from
@node-core/website-i18n
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packages/i18n/src/index.mjs |
Removed importLocale function and converted getter functions to constant exports |
docs/technologies.md |
Removed documentation section for the deleted next.locales.mjs file |
apps/site/tests/e2e/general-behavior.spec.ts |
Replaced importLocale with direct dynamic import and static import for English locale |
apps/site/next.rewrites.mjs |
Updated import to use @node-core/website-i18n directly |
apps/site/next.locales.mjs |
Deleted wrapper file that re-exported i18n functions |
apps/site/next.dynamic.page.mjs |
Updated imports to source from @node-core/website-i18n |
apps/site/next.dynamic.mjs |
Updated imports to source from @node-core/website-i18n |
apps/site/next-data/generators/downloadSnippets.mjs |
Updated import to use @node-core/website-i18n directly |
apps/site/navigation.mjs |
Updated import to use @node-core/website-i18n directly |
apps/site/middleware.ts |
Updated imports to source from @node-core/website-i18n |
apps/site/i18n.tsx |
Replaced importLocale with direct dynamic import and updated other imports |
apps/site/components/withNavBar.tsx |
Updated import to use @node-core/website-i18n directly |
apps/site/components/withMetaBar.tsx |
Updated import to use @node-core/website-i18n directly |
apps/site/components/withDownloadSection.tsx |
Updated import to use @node-core/website-i18n directly |
apps/site/app/sitemap.ts |
Updated imports to source from @node-core/website-i18n |
apps/site/app/[locale]/page.tsx |
Updated imports to source from @node-core/website-i18n |
apps/site/app/[locale]/next-data/og/[category]/[title]/route.tsx |
Updated import to use @node-core/website-i18n directly |
apps/site/app/[locale]/layout.tsx |
Updated imports and replaced availableLocalesMap with inline find() operation |
apps/site/app/[locale]/feed/[feed]/route.ts |
Updated import to use @node-core/website-i18n directly |
apps/site/app/[locale]/download/archive/[version]/page.tsx |
Updated import to use @node-core/website-i18n directly |
apps/site/app/[locale]/blog/[...path]/page.tsx |
Updated import to use @node-core/website-i18n directly |
apps/site/app/[locale]/[...path]/page.tsx |
Updated imports to source from @node-core/website-i18n |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
I don't think OP understands the side effects of their changes.
Please read next-intl docs and triple check you're not breaking things unintentionally...
Edit: the phrasing above was poor, I meant to say that Im not confident of the changes and that OP has checked if every next-intl feature is still working as expected. I absolutely believe on OP's skill and just wanted to ensure that they verify that everything is still working as expected
ovflowd
left a comment
ovflowd
left a comment
There was a problem hiding this comment.
SGTM! Appreciate for triple checking on my concerns.
Currently, per Next.js's bundle analyzer, all locales are sent to the client.
This PR removes the
importLocalefunction, as I suspect it was the culprit.