feat: Preserve locale with cookie & react-intl#58
feat: Preserve locale with cookie & react-intl#58canerakdas wants to merge 3 commits intonodejs:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Updates the top navigation to derive Reviewed by Cursor Bugbot for commit 56616b9. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2c817f8. Configure here.
There was a problem hiding this comment.
Pull request overview
Adds client-side locale detection (via NEXT_LOCALE) and uses it to localize navigation links from the Learn site back to locale-prefixed nodejs.org routes.
Changes:
- Introduces a
LocaleProvider(react-intlIntlProvider) that detects locale from theNEXT_LOCALEcookie. - Adds a link-localization utility and updates the navigation component to rewrite locale-prefixed links.
- Updates
site.jsonnavigation entries to include the default locale prefix and addsreact-intlas a dependency.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| util/link.js | New helper to rewrite locale-prefixed internal links. |
| providers/LocaleProvider.jsx | New provider to source locale from NEXT_LOCALE and expose it via react-intl. |
| components/Navigation/index.jsx | Uses detected locale to localize nav item links. |
| components/Layout/index.jsx | Wraps page layout with LocaleProvider. |
| site.json | Adds defaultLocale and updates nav links to include /en/ prefixes. |
| package.json | Adds react-intl dependency. |
| package-lock.json | Locks new react-intl and related FormatJS dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import MetaBar from '../Metabar'; | ||
| import SideBar from '../Sidebar'; | ||
| import Footer from '../Footer'; | ||
| import LocaleProvider from '../../providers/LocaleProvider'; |
There was a problem hiding this comment.
nit: providers folder inside components folder.
| const [navigationItems, setNavigationItems] = useState(navigation); | ||
| const { locale } = useIntl(); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
Do we need to use an useEffect here? Can't this be an useMemo?
| * @param {string|null} locale - The locale to apply to the link. | ||
| * @returns {string} - The localized link. | ||
| */ | ||
| export const localizeLink = (link, locale) => { |
There was a problem hiding this comment.
I believe react-intl has utils for localizing a link.
There was a problem hiding this comment.
That's one of the intentions behind using react-intl, is to have it localizing a link for you. Although I'm not seeing now within react-intl a way of doing so. 🤔
There was a problem hiding this comment.
I don't think such a feature exists 🙈 It seems like we're currently only using a provider that stores the locale. To be honest, I'm not sure there’s much benefit to using it this way 🥲
There was a problem hiding this comment.
Does react-intl increase our bundle a lot?
There was a problem hiding this comment.
Yeah, I chatted with @canerakdas in DMs he's going to pivot without react-intl.
This PR aims to preserve the current language across navigation between nodejs.org and learn pages if the
NEXT_LOCALEcookie is present.