Yes
Yes
Platform Agnostic
Latest Docusaurus (master branch)
cd website
yarn start
docs/getting-started-installation.md title toInstallation asdfyarn startThis happen after PR #710
Prior to this, i18n/en.json is always newly generated
After this, i18n/en.json will persist and override newly generated i18n/en.json
Hence, title is not changing
But PR #710 is correct because it fixes bug from #158 that allow user allow overriding of english strings
Delete i18n/en.json when changing title
Should we just always delete i18n/en.json and then regenerate when starting server.js or running generate.js?
What is intended from PR #158 is that user can override Docusaurus provided strings that a project may not like (for example ReasonReact would like to say "Edit" instead of "Edit this Doc" in English). So they can modify i18en.json directly
Related code:
https://github.com/facebook/Docusaurus/blob/fa20c93d5c1b5a9369cf1c5909955fc58f0dcb8e/lib/write-translations.js#L27-L31
But this introduces this bug because if I made a change to my title, old title from i18en.json exist & overrides i18en.json new title
If we always delete i18en.json that is just the same as not allowing to override strings and effectively deleting this code:
https://github.com/facebook/Docusaurus/blob/fa20c93d5c1b5a9369cf1c5909955fc58f0dcb8e/lib/write-translations.js#L27-L31
That's why there has been no issue with that code before #710 since October 2017
Hi @rickyvetter,
Just wanted to ask what is the intended / right behavior should be like when you implement #158 ?
If we always delete i18en.json that is just the same as not allowing to override strings and effectively deleting this code
I guess that's true. Hmmm.... gotta think about this a bit.
Thanks for the analysis @endiliey! Very thorough. You're correct in my intent. The goal was to allow overriding of all Docusaurus provided strings with project-specific defaults. I think those Object.assigns should probably be removed in favor of explicit checking.
From a chat with @rickyvetter....
Delete - https://github.com/facebook/Docusaurus/blob/fa20c93d5c1b5a9369cf1c5909955fc58f0dcb8e/lib/write-translations.js#L158-L164
The custom lines above that should check if it exists in the file before writing.
I just checked Docusaurus repo & our documentation website at https://docusaurus.io/docs/en/translation.html
Since the files are generated, you do not need to have any files in your website/i18n or ? website/translated_docs directory as part of your repo. So you can can add website/i18n/* and website/translated_docs to your .gitignore file.
Found out that i18n/en.json is always ignored through .gitignore.
https://github.com/facebook/Docusaurus/blob/master/.gitignore
So actually the piece of code I mentioned has never been used at all. Should we just delete the ability to override the string once and for all ?
This means we will delete
https://github.com/facebook/Docusaurus/blob/fa20c93d5c1b5a9369cf1c5909955fc58f0dcb8e/lib/write-translations.js#L27-L31
The user didn't even know that they can even override the string unless they look at Docusaurus code directly.
I can assure you that this is the safest choice, it removes this bug, we have lesser code, and it won't break any user website.
Hi @endiliey While we don't check in the en.json file into Docusuarus, it is used when doing the uploads to Crowdin for translations. write-translations.js creates and uses that file for that purpose.
@joelmarcey that's true, but isn't it always a newly generated i18n/en.json since there is no local i18n/en.json in GitHub?
Based on
https://github.com/facebook/Docusaurus/blob/d28b864a59fabeea45add8c090a13de7d0530de5/.circleci/config.yml#L75-L87
We always publish through continuous integration (e.g circle CI) in which it uses the github repo as source of truth and there is no i18n/en.json when write-translations is executed, so it generates new i18n/en.json & upload it to crowdin.
So this code is always false in production
https://github.com/facebook/Docusaurus/blob/fa20c93d5c1b5a9369cf1c5909955fc58f0dcb8e/lib/write-translations.js#L27-L31
We never have an existing i18n/en.json to override the default strings. That's why this bug doesnt exist in production, only in development.
I might misunderstood it. Let me know if i am.
same issue here.
I don't want to put the showstopper label on this, but this is a very high-pri bug as 1.2.0 brings it out into plain view for people.
This should be the next thing fixed before anything else, imho.
I will look at this some tomorrow. If anyone can look beforehand, that would be great. Even if, short-term, we have to break allowing custom strings to make sure people can modify the header metadata (e.g. title), I think we should consider that.
I have few alternatives
custom-en.jsonif (fs.existsSync(CWD + '/i18n/custom-en.json')) {
currentTranslations = JSON.parse(
fs.readFileSync(CWD + '/i18n/custom-en.json', 'utf8')
);
}
If this is the case we might need to un- git ignore custom-en.json & update the docs
@endiliey
I think I kinda like option #2 after giving it some thought today. What we can do is load the generated i18n/en.json first and and get that in the currentTranslations array. Then load i18n/custom-en.json and either override current values in the array if there would be a duplicate or append to the array.
What do you think?
Sounds great @joelmarcey.
Are you going to work on it ? Otherwise, I can submit a PR when I'm back roughly after this week.
I am going to give it a run - try to get a PR before the weekend.
@endiliey @JoelMarcey Can I try to fix this bug 馃槃
Hi @vaibhavsingh97 - I have a PR #775 out for this already. If you would like to have a look and provide any additional suggestions, that would be welcome 馃槃
Most helpful comment
What is intended from PR #158 is that user can override Docusaurus provided strings that a project may not like (for example ReasonReact would like to say "Edit" instead of "Edit this Doc" in English). So they can modify
i18en.jsondirectlyRelated code:
https://github.com/facebook/Docusaurus/blob/fa20c93d5c1b5a9369cf1c5909955fc58f0dcb8e/lib/write-translations.js#L27-L31
https://github.com/facebook/Docusaurus/blob/fa20c93d5c1b5a9369cf1c5909955fc58f0dcb8e/lib/write-translations.js#L158-L164
But this introduces this bug because if I made a change to my title, old title from
i18en.jsonexist & overridesi18en.jsonnew titleIf we always delete
i18en.jsonthat is just the same as not allowing to override strings and effectively deleting this code:https://github.com/facebook/Docusaurus/blob/fa20c93d5c1b5a9369cf1c5909955fc58f0dcb8e/lib/write-translations.js#L27-L31
That's why there has been no issue with that code before #710 since October 2017