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

Chore/add warning on invalid fallback lang #2241

Conversation

AlemTuzlak
Copy link

I've tried to implement the warning for the issue outlined in the ticket below, although I am not sure about the following:

  1. How do I test this and where? There are a lot of test files and I don't know what the best place is and what to use (mocks, spies etc)
  2. Is this if check okay or could I have used optional chaining? I am not sure so I just checked if it exists and has a length
  3. should the "dev" be ignored?
    react-i18next:: i18n.languages were undefined or empty react-i18next#1803

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • x] run tests npm run test
  • tests are included
  • commit message and code follows the Developer's Certification of Origin

Checklist (for documentation change)

  • only relevant documentation part is changed (make a diff before you submit the PR)
  • motivation/reason is provided
  • commit message and code follows the Developer's Certification of Origin

@coveralls
Copy link

Coverage Status

coverage: 96.09% (-0.1%) from 96.199%
when pulling 168e300 on AlemTuzlak:chore/add-warning-on-invalid-fallback-lang
into f2a6b6c on i18next:master.

@adrai
Copy link
Member

adrai commented Sep 24, 2024

  1. Create a new test file and pass in a logger dummy like here: https://github.com/i18next/i18next/blob/master/test/runtime/i18next.hasLoadedNamespace.test.js#L4
  2. if check is ok... no optional chaining
  3. why should dev be ignored...

fallbackLng can be a string an array or an object: https://www.i18next.com/principles/fallback#fallback-to-different-languages

@AlemTuzlak
Copy link
Author

Some tests pass "dev" as a fallback even though it's not in supported languages so I wasn't sure if there was some behavior I'm not aware of, okay will fix what you just said!

@adrai
Copy link
Member

adrai commented Sep 24, 2024

Some tests pass "dev" as a fallback even though it's not in supported languages so I wasn't sure if there was some behavior I'm not aware of, okay will fix what you just said!

ok, keep the exception with the dev language

@AlemTuzlak
Copy link
Author

so I found something interesting, while trying to write the tests I actually got 2 logs from module called languageUtils that warned that the default language is not in the supported languages list, it seems to me that the feature is already there, maybe it's not called in the react package?

@adrai
Copy link
Member

adrai commented Sep 27, 2024

totally forgot that... yes... it's already there ;-)

Yes, it should be called also via react-i18next...

@adrai
Copy link
Member

adrai commented Sep 28, 2024

Just tested here (change the fallbackLng accordingly): https://codesandbox.io/p/sandbox/react-i18next-http-example-forked-n4fqct

seems to work:
image

So this PR can be closed, right?

@AlemTuzlak
Copy link
Author

I'll have to investigate this on my side, maybe it just doesn't trigger in Remix for some remix related reason. I'll raise the issue again if I figure out the root cause

@AlemTuzlak AlemTuzlak closed this Oct 23, 2024
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.

3 participants