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

I18next action #80

Closed
wants to merge 9 commits into from
Closed

Conversation

JohnRDOrazio
Copy link
Collaborator

After many hours of testing, I finally came up with a solution that will work both from the command line, or from a Github workflow. I also took the opportunity to update to Node 20, if you think that's ok?

  1. Need to install chalk explicitly among the dev-dependencies blocked at v4 because v5 changed to ES module syntax, but it seems that i18next-scanner doesn't have the right version as a dependency because it was pulling in v5, effectively breaking the scanner configuration
  2. We previously had a little situation that was never quite fixed: if a new <Trans> component is placed in the code, when the scanner picked it up, it would use the default value (the text contents of the component) for both English and non English language files, which is not very desirable: you don't want to populate non English language files with English values. You only need the keys, but with empty values. I finally came up with a solution: use two separate configuration scripts, one that will update only the English language file and will populate keys related to <Trans> components with the default value; the other configuration script that will update all other non English language files and will populate keys related to <Trans> components with an empty value.
  3. In order to automate this process I went ahead and created a Github workflow action that will trigger on push to main if any .js or .jsx files are changed.
  4. I also set sort to true, so that keys will be sorted alphabetically. This will ensure that:
    • they are easy to navigate and find in the language files
    • they will all be in the same order in every language file

So far all is tested and working well, except maybe one little thing in the Github workflow action: it seems that a Husky hook is trying to run when the action tries to commit changes, but it can't find Husky. I don't know much about husky, how or where should it be installed inside the Github action?

@JohnRDOrazio JohnRDOrazio added enhancement New feature or request maintainability Ways to make sure the code is maintainable by the community labels Jun 24, 2024
Copy link

netlify bot commented Jun 24, 2024

Deploy Preview for confessit-web ready!

Name Link
🔨 Latest commit d5b8ccb
🔍 Latest deploy log https://app.netlify.com/sites/confessit-web/deploys/6678f904850bb00008cff13a
😎 Deploy Preview https://deploy-preview-80--confessit-web.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kas-catholic
Copy link
Owner

NodeJS 20.x sounds great, but I'd prefer to merge it in a separate commit/PR for numerous reasons. Would you like to make a separate PR for that? If not I'm happy to do it myself since you've done so much other work here, I could get it done quickly.

For i18next-scanner, I like your solution. Great idea to use separate configs! But I'm very hesitant about the auto-commit workflow. It's not that I don't think it will work, but that it's not the way I'm most comfortable working. I'd generally prefer, for a variety of reasons, to make the translations file changes in the same commit where the code changes. Fortunately, I actually think we can do this somewhat easily. Husky is a tool to automate the setup of git hooks that we're already using for a linting hook with Prettier. I think (hope) it would be easy to move your script out of the GitHub Action and instead run it as a hook when making a git commit, to include the translation changes as part of the same commit. I'd also be happy to work on doing this, using this PR as a starting point, or you can do the work yourself if you prefer. Let me know what you'd like me to help with.

@kas-catholic
Copy link
Owner

And of course, thanks for all your help with Translations! With that AI commit you did, we have very complete translations in several languages now!

@JohnRDOrazio
Copy link
Collaborator Author

JohnRDOrazio commented Jun 25, 2024

NodeJS 20.x sounds great, but I'd prefer to merge it in a separate commit/PR for numerous reasons.

Sure I can do that.

I actually think we can do this somewhat easily. Husky is a tool to automate the setup of git hooks that we're already using for a linting hook with Prettier. I think (hope) it would be easy to move your script out of the GitHub Action and instead run it as a hook when making a git commit, to include the translation changes as part of the same commit.

I'm not familiar with Husky, though I'm interested in getting to know it. But best you take care of that. We could close this PR and I'll let you transport the config scripts in a new PR with a Husky hook pulling the sleigh 😄

@JohnRDOrazio
Copy link
Collaborator Author

Is it just a question of adding the yarn dlx i18next-scanner calls to the husky pre-commit file, perhaps just before npx lint-staged ?

npx lint-staged

@kas-catholic
Copy link
Owner

Yes, that should be it actually. Take a look at #86 and see if it seems good to you. (Or we can rebase and merge this PR if you prefer, I don't mind either way.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request maintainability Ways to make sure the code is maintainable by the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants