Skip to content
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

feat(core): add i18n.setCatalogAndActivate for easier nextjs integration #1541

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

thekip
Copy link
Collaborator

@thekip thekip commented Mar 21, 2023

Description

Fixes #1339

Integration with nextjs is not easy as it could be. You can check how much non-trivial code should be written https://github.com/lingui/swc-plugin/pull/38/files to make it work without accessing private properties of i18n.

This PR brings special method which

  • set all data in one call. More aligned with how users really use it now.
  • allow to set notify: false which is crucial to make integration easy.

The problem is in the race condition. When you setting locale or messages i18n emits an event. For that moment of time LinguiProvider already initialized and subscribed to the event from i18n. The emitting of this event happened inside react's "reconciliation phase" and it causes another setState() inside LinguiProvider which is not allowed during this phase.

Simply disabling emitting event in this method fixes the issue and simplify integration a lot.

So instead

export function useLinguiInit(messages: Messages) {
  const router = useRouter()
  const locale = router.locale || router.defaultLocale!
  const firstRender = useRef(true)

  // run only once on the first render (for server side)
  if (messages && firstRender.current) {
    i18n.load(locale, messages)
    i18n.activate(locale)
    firstRender.current = false
  }

  useEffect(() => {
    if (messages) {
      i18n.load(locale, messages)
      i18n.activate(locale)
    }
  }, [locale, messages])

  return i18n;
}

User have to write:

export function useLinguiInit(messages: Messages) {
  const router = useRouter()
  const locale = router.locale || router.defaultLocale!
  i18n.loadAndActivate(locale, messages, false);

  return i18n;
}

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist

  • I have read the CONTRIBUTING and CODE_OF_CONDUCT docs
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@vercel
Copy link

vercel bot commented Mar 21, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
js-lingui ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 21, 2023 at 10:39AM (UTC)

@github-actions
Copy link

github-actions bot commented Mar 21, 2023

size-limit report 📦

Path Size
./packages/core/build/esm/index.js 1.57 KB (0%)
./packages/detect-locale/build/esm/index.js 812 B (0%)
./packages/react/build/esm/index.js 1.59 KB (0%)
./packages/remote-loader/build/esm/index.js 7.25 KB (0%)

@codecov
Copy link

codecov bot commented Mar 21, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.16 🎉

Comparison is base (10633f3) 75.17% compared to head (bd20146) 75.34%.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #1541      +/-   ##
==========================================
+ Coverage   75.17%   75.34%   +0.16%     
==========================================
  Files          78       78              
  Lines        1962     1971       +9     
  Branches      512      515       +3     
==========================================
+ Hits         1475     1485      +10     
+ Misses        376      375       -1     
  Partials      111      111              
Impacted Files Coverage Δ
packages/core/src/i18n.ts 66.66% <100.00%> (+7.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@andrii-bodnar andrii-bodnar merged commit 3659dec into lingui:next Mar 21, 2023
@thekip thekip deleted the setCatalogAndActivate branch March 21, 2023 12:29
@vonovak
Copy link
Collaborator

vonovak commented Mar 21, 2023

hello 👋
quick questions:

  1. why is this doing this: this._messages[this._locale] = messages as opposed to running this._load(this_locale, messages)? The docs say "This method is i18n.load() + i18n.activate() in one pass." but that's not 100% true in fact. I'd say the code should stay but the docs should be more accurate?

  2. if the "change" event is not emitted, how does the integration with the I18nProvider work?

    const updateContext = () => {

thanks :)

@thekip
Copy link
Collaborator Author

thekip commented Mar 21, 2023

@vonovak look at changes in this integration in nextjs https://github.com/lingui/swc-plugin/pull/38/files

why is this doing this: this._messages[this._locale] = messages as opposed to running this._load(this_locale, messages)? The docs say "This method is i18n.load() + i18n.activate() in one pass." but that's not 100% true in fact. I'd say the code should stay but the docs should be more accurate?

i did not get the question

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants