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

Fix react-i18next(and next-i18next) framework integration #735

Merged
merged 2 commits into from
Sep 8, 2022

Conversation

SimeonC
Copy link
Contributor

@SimeonC SimeonC commented Mar 1, 2022

Fixing the react-i18next framework to extend the base i18next framework as there shouldn't be many differences between them.

Also added in the v4 plurals fixing #677 and #688

In the image below, i18n-a11y is detecting the file if I use a . but not if I use a : (the : here is correct as it's a namespaced file)
CleanShot 2022-03-01 at 17 57 17

@ThenTech
Copy link

Can this be approved and merged? Having #677 and #688 would be good.

@Arno500
Copy link

Arno500 commented May 31, 2022

Hello @SimeonC
Any updates on this?
Thanks for the fix :P

@SimeonC
Copy link
Contributor Author

SimeonC commented May 31, 2022

I'm unfortunately powerless to merge this. As far as I'm aware it's all correct and ready to merge.

@Arno500
Copy link

Arno500 commented May 31, 2022

Oh alright, I thought it was to you to merge this. Sorry for the noise…

@Stefouch
Copy link

Can this be approved and merged? Having #677 and #688 would be good.

Get my upvote for this merge!
I have the same issue

@leon0399
Copy link

leon0399 commented Sep 6, 2022

+1 for merge

Is project dead?

@beinarovic beinarovic self-requested a review September 8, 2022 09:24
@beinarovic beinarovic merged commit 80de119 into lokalise:main Sep 8, 2022
@boredland
Copy link

Has this change been released? I am a bit confused, as for me v4-style plurals don't seem to work and #688 is still open.

image

image

@aranard
Copy link
Contributor

aranard commented Dec 9, 2022

Is this project still being actively maintained? I'd like to contribute a feature that will help my team, and I'm running into issues getting the extension to build.

This seems to be in part because of this change being merged. The splitting of frameworks/react.ts into two separate files (react-i18next.ts and react-intl.ts) has broken code in frameworks/index.ts because that original file no longer exists (but is still being referenced):

image

Is this something anyone is actively aware of or looking into? Thanks.

import { I18nextFramework } from './i18next'
import { LanguageId } from '~/utils'

class ReactI18nextFramework extends Framework {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR does not seem to have been checked at all. Surely ReactI18nextFramework should be extending I18nextFramework and not Framework. I18nextFramework is also a default export and not a named one...

@Swiftwork
Copy link
Contributor

This whole PR is broken and adds nothing to the extension except for supporting pluralization. It breaks imports, has the wrong usage patterns, and doesn't even add itself to the frameworks list. Please revert this PR and add the pluralization to the i18next framework instead.

'{key}_zero',
'{key}_one',
'{key}_two',
'{key}_few',
'{key}_many',
'{key}_other'

@graup
Copy link

graup commented Dec 20, 2022

I've found maintainers only reply here if you send them a request through Lokalise customer support.

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

Successfully merging this pull request may close these issues.