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: add support for JSONv4 #214

Merged
merged 4 commits into from
May 1, 2022
Merged

feat: add support for JSONv4 #214

merged 4 commits into from
May 1, 2022

Conversation

gilbsgilbs
Copy link
Owner

Also force using JSONv4 for ICU as it makes the plural categories more
correct. This is technically a breaking change, but as the feature was
explicitely marked as expermiental, I wouldn't worry too much.

I have yet to decide if JSONv4 should be made the default now (which
would require a major version increase). We surely want new users to use
JSONv4 by default, but a major version increase might be an opportunity
to include other changes.

closes #203

@gilbsgilbs gilbsgilbs force-pushed the jsonv4 branch 3 times, most recently from f54275d to 8948775 Compare May 1, 2022 00:20
gilbsgilbs added 3 commits May 1, 2022 18:39
Also force using JSONv4 for ICU as it makes the plural categories more
correct. This is technically a breaking change, but as the feature was
explicitely marked as expermiental, I wouldn't worry too much.

I have yet to decide if JSONv4 should be made the default now (which
would require a major version increase). We surely want new users to use
JSONv4 by default, but a major version increase might be an opportunity
to include other changes.

closes #203
The plural count changed between node 12 and node 14, which makes the
test irrelevantly fail for node 12.
@gilbsgilbs gilbsgilbs merged commit a01d802 into master May 1, 2022
@gilbsgilbs gilbsgilbs deleted the jsonv4 branch May 1, 2022 17:28
@pederjohnsen
Copy link

@gilbsgilbs any idea when this will get released? 🙏🏻

@gilbsgilbs
Copy link
Owner Author

gilbsgilbs commented Jul 6, 2022

Sorry @pederjohnsen , I've been busy lately. The reason I did not publish a release yet is that I am not ready to undergo a wave of reports in case the release isn't as stable and harmless as I wished. Therefore, it would really help if you tested current master branch on your projects and confirm it indeed works. It would definitely boost my confidence and help me make the release faster 😬 . Otherwise, I'll try to draft a release by next week or so (no guarantee).

@Regaddi
Copy link

Regaddi commented Jul 20, 2022

Hey @gilbsgilbs,

Sorry to dig up this thread again.
What is the best way forward to test your current master branch?
I tried installing the package per "babel-plugin-i18next-extract": "github:gilbsgilbs/babel-plugin-i18next-extract#master", but that only clones your repository into node_modules, but it's not transpiled nor does it match the configured entry files as defined in your package.json.
I assume I'd need to configure something to convert the cloned repo to the expected structure, as published to npm.

Alternatively, would you feel confident enough to publish an alpha release to npm? This would simplify testing a lot ☺️

@gilbsgilbs
Copy link
Owner Author

gilbsgilbs commented Jul 20, 2022

Thanks @Regaddi :)

What is the best way forward to test your current master branch?

I think the easiest way is to clone the repo and build it locally (yarn run build), then link it (yarn link && cd /path/to/YOUR/project && yarn link babel-plugin-i18next-extract). Not sure if the linking procedure is exactly the same with NPM and others though, so please adapt depending on your tools.

Alternatively, would you feel confident enough to publish an alpha release to npm? This would simplify testing a lot relaxed

Yes, you're absolutely right. Thanks for bringing up this idea. I'll try to do that tonight.

@pederjohnsen
Copy link

@gilbsgilbs sorry, been on holiday and only seen your reply 🤦🏻

I'm also happy to give it a test if you publish an alpha release 🤝

@gilbsgilbs
Copy link
Owner Author

@Regaddi
Copy link

Regaddi commented Jul 21, 2022

@gilbsgilbs Thanks so much for setting this up, this really simplifies testing by a lot 🙌

I already started writing down a few findings, but then I figured I might have to clean my node_modules, which in fact did the trick. Keeping my notes as a hint for others anyway:

Before cleaning node_modules

From a quick test-run I'm noticing the following, probably unexpected, behaviors:

Unexpected reappearing of keys

For some reason babel-plugin-i18next-extract seems to hold to old keys in memory or somewhere stored on disk. I used to have a key in use called audio.microphone_{number} but recently replaced it with audio.microphoneWithNumber. Now when running babel with the plugin, all of the sudden the key audio.microphone_{number} reappears in my translation file 🤔

Unexpected plural keys

When adding a new key with plurals, e.g. t('somethingWithCount', { count: aNumber }), the plugin creates 4 keys:

  • somethingWithCount
  • somethingWithCount_one
  • somethingWithCount_other
  • somethingWithCount_plural
    Given that we configured to compatibilityJSON: 'v4', I'd only expect the _one and _other key to appear, or is that a wrong assumption? We've already migrated to i18next 21, so only _one and _other is effectively in use and we'd have to manually remove somethingWithCount and somethingWithCount_plural.

After cleaning node_modules

All works as expected 🙌 This is an amazing step forward and I'm really stoked that it just works. The underlying codebase covers >250 TypeScript and JavaScript files with ~28.000 lines of code, in case that's a helpful metric for you.

I'll update to the new version now and keep you posted about any issues, if any pop up.

Thank you so much for your work!
IMO this is by far the most convenient and easiest-to-apply solutions, compared to other i18n scanners. 🙇

@gilbsgilbs
Copy link
Owner Author

Thanks for your precious feedback @Regaddi . I really appreciate that.

The issues you had before cleaning up node_modules were weird to say the least, but I'll advise users to cleanup their node_modules in the changelog then. After all, it's been a while since the last release and the dependencies have moved a lot since then.

I don't think this plugin stores anything on disk (except the output file). And I'll also mention that the plugin won't migrate the files to v4 for you, pointing to i18next's migration guide and i18next-v4-format-converter (even though that seemed unrelated to your issue since you said you had already migrated to v4).

IMO this is by far the most convenient and easiest-to-apply solutions, compared to other i18n scanners.

Thank you! This is the exact reason why I initially started this project, so these kind words are especially meaningful to me.

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.

Support i18next JSON v4's format
3 participants