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

i18n Setup : Simpler configuration + examples #1847

Merged
merged 36 commits into from
Mar 9, 2021
Merged

i18n Setup : Simpler configuration + examples #1847

merged 36 commits into from
Mar 9, 2021

Conversation

simoncrypta
Copy link
Collaborator

@simoncrypta simoncrypta commented Feb 24, 2021

This is to make internalization easy with Redwood

  • Replaces the i18n.js.template with a simpler and minimal configuration with examples.
  • Show a configuration for English and French translation that can easily be changed.
  • Show example with a page in the comment on i18n.js file
  • Error handling inspire by the tailwind setup command
  • fixes yarn rw setup i18n fails  #1590

what's next

I think we should make a cookbook for translation at scale with RedwoodJS; I will try to work on it soon.

We could make it work with Prerender, It's currently don't work because the useTranslate hook I guess.

We will need to make a way with Redwood Router to add the language inside the URL.

@github-actions
Copy link

github-actions bot commented Feb 24, 2021

📦 PR Packages

Click to Show Package Download Links

https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/create-redwood-app-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-api-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-api-server-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-auth-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-cli-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-core-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-dev-server-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-eslint-config-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-eslint-plugin-redwood-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-forms-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-internal-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-prerender-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-router-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-structure-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-testing-0.26.2-06e129d.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/1847/redwoodjs-web-0.26.2-06e129d.tgz

Install this PR by running yarn rw upgrade --pr 1847:0.26.2-06e129d

@dac09
Copy link
Contributor

dac09 commented Mar 1, 2021

Hey @simoncrypta, thank you for this! Do you think we can get this out of draft soon? To help you with your outstanding points:

  • check if if x already exists
    If you use the writeFIle method, and pass the "force" flag to overwriteExisting (like you have in some places) that should cover it

  • I don't know why but new files in the templates folder cannot be found in my test
    Not sure why this is happening. Are you running build:watch or build before runing test?

@simoncrypta
Copy link
Collaborator Author

simoncrypta commented Mar 1, 2021

Hey @dac09 , Thank you for your help! I currently search for the best way to do internalization with RedwoodJS, and I have more research to do. That's why I didn't get this out of draft yet. I plan to make internalization easy with Redwood with a good convention because they have so many ways to do it with i18n.

Not sure why this is happening. Are you running build:watch or build before runing test?

I running build:watch and everything updates except for what is on the templates folder. I would try again and write more details if isn't working again.

@thedavidprice
Copy link
Contributor

@simoncrypta possibly helpful reference article from the Forums: https://community.redwoodjs.com/t/article-about-i18n-and-l10n/1477

And thanks for working on this. Huge help!

@simoncrypta simoncrypta marked this pull request as ready for review March 3, 2021 20:22
@simoncrypta
Copy link
Collaborator Author

simoncrypta commented Mar 3, 2021

@dac09 I think you could maybe help me to make it work with prerendering. i18next have a doc for SSR, but don't understand where they put their code for Passing initial translations / initial language down to client 😅

@thedavidprice
Copy link
Contributor

@clairefro curious if you have availability to help review this one, specifically about high-level setup and if this includes enough foundational elements to get a project started.

@clairefro
Copy link
Contributor

clairefro commented Mar 3, 2021

Cool! I will take a closer look at this tonight

I currently search for the best way to do internalization with RedwoodJS, and I have more research to do.

In the meantime @simoncrypta , Gatsby might be an interesting reference for how a framework handles i18n
https://www.gatsbyjs.com/docs/how-to/adding-common-features/localization-i18n/

@simoncrypta
Copy link
Collaborator Author

simoncrypta commented Mar 5, 2021

Thank you, @thedavidprice, to fix my mistakes.

Very interesting i18n RFC @clairefro ! Sub-path routing is definitely something essential that I didn't completely convert with i18next-browser-languagedetector. Also, I didn't think of the html lang attribute, and I presume it is not something too hard to cover; I will work on it!

Copy link
Contributor

@thedavidprice thedavidprice left a comment

Choose a reason for hiding this comment

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

I had a couple things I wanted to test locally (e.g. if the "sync Yarn" step was necessary as it was for Tailwind; in this case it was not). And also was generally curious about how React i18n works. I found a path bug I fixed and also updated some vars to match the switch to App.js. Overall was in great shape and works really well. See my simple changes in the commits for reference.

I have one comment about where to place the i18n.js file import in App.js. Not a dealbreaker but please check it out.

@thedavidprice
Copy link
Contributor

thedavidprice commented Mar 5, 2021

Thanks for the review and suggestions @clairefro Helpful indeed!

And great work here @simoncrypta This is a really helpful foundational, first step to get people up and running (and not just because it fixes the current bug).

I'll hand off to @dac09 for merge timing, but this has my approval.

Per Claire's suggestion:

I'm guessing this will rely on prerender. In addition to obvious things like localized strings for description, title etc, ideally the html lang attribute (ex: ) and hreflang attribute should be set per locale to promote discoverability.

SEO is an important addition to this initial setup and does not require prerender to get started. Using something like React Helmet, we might be able to add a few pieces to this foundation that addresses both HTML lang and hreflang attributes.

Might you be interested in exploring this as a next step here via a PR, @simoncrypta ?

@clairefro
Copy link
Contributor

SEO is an important addition to this initial setup and does not require prerender to get started. Using something like React Helmet, we might be able to add a few pieces to this foundation that addresses both HTML lang and hreflang attributes.

Even with React Helmet, won't prerender be necessary in order to support web crawlers for search engines and social shares?

@thedavidprice
Copy link
Contributor

Even with React Helmet, won't prerender be necessary in order to support web crawlers for search engines and social shares?

There's a lot of misconceptions here, so it's more complicated/confusing than it needs to be (which is partly intentional by Google). Crawlers render JS just fine and have for a while, so that's not really an issue. BUT site performance (alas, Lighthouse...) plays a role in the search ranking performance of a website. Meaning that even if your SEO is getting indexed just fine, your site might have terrible (or amazing) end-user performance — Google will adjust where you site displays in search results accordingly. There's a lot of tricks to get sites to perform. One of them is serving static, i.e. cached, content.

Social Shares depend on OG data in the page meta, which does require being rendered server-side.

Messy mess.

tl;dr:
Getting started with React Helmet will indeed improve SEO as is even without pre-rendered pages, which is a great start.

@thedavidprice
Copy link
Contributor

Great work on all this @simoncrypta 🚀

Would you be interested in opening a new Issue to discuss possible improvements regarding React Helmet + SEO?

@thedavidprice thedavidprice added this to the next release milestone Mar 9, 2021
@thedavidprice thedavidprice merged commit 42ec5eb into redwoodjs:main Mar 9, 2021
@simoncrypta simoncrypta deleted the fix-i18n branch March 9, 2021 04:13
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.

yarn rw setup i18n fails
4 participants