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

Refactor of Build Process #360

Conversation

neilsutcliffe
Copy link
Contributor

@neilsutcliffe neilsutcliffe commented Dec 27, 2017

Progress towards #336 and #325 as we need to unify the render functions between the server and build process. ATM they are two completely parallel processes.

Whilst this PR doesn't actually solve that problem, it sets the render functions up nicely to be called from the server.

I've also commented some extra areas which need some attention.

The jest tests for build all pass on Windows, and I tried with multiple languages. One thing that did fail was setting English disabled and enabling another language. That may be an existing issue however.

Update: Build now succeeds on UNIX

- Now uses default.json. Existing sites will continue to work
- Removed single use of PropType property
- Removed languages array from translation class. use the env.js file instead.
- We now default to an empty list of languages.
…s. The render classes can be later used in the server.js file. Step 1 of making our logic DRY.
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Dec 27, 2017
@neilsutcliffe
Copy link
Contributor Author

Can reproduce build error locally now, fixing it up.

@neilsutcliffe
Copy link
Contributor Author

Woo! Tests are all working.

neilsutcliffe and others added 9 commits December 27, 2017 22:22
)

* Do not set html lang attribute if there is no language set

* Prettier fixes
Now renders all languages (only did first), and library static files (didn't copy Docusaurus static files)
Moved to relative links to avoid having to do the whole mdToHtml stuff. Opinions?
- Now uses default.json. Existing sites will continue to work
- Removed single use of PropType property
- Removed languages array from translation class. use the env.js file instead.
- We now default to an empty list of languages.
…s. The render classes can be later used in the server.js file. Step 1 of making our logic DRY.
…d translation files after (so we can catch it locally)
Now renders all languages (only did first), and library static files (didn't copy Docusaurus static files)
Moved to relative links to avoid having to do the whole mdToHtml stuff. Opinions?
@neilsutcliffe
Copy link
Contributor Author

neilsutcliffe commented Jan 31, 2018

https://xkcd.com/323/

I've rebased and made the CSS use the shared library. This makes the upcoming theme support easier.

Also I super recommend GitKraken. It made the rebase much easier than I thought it would.

@JoelMarcey
Copy link
Contributor

@neilsutcliffe Hi Neil! I think it is time we start looking at bringing some of this refactor in soon. If you are still interested, can you do a clean rebase - there shouldn't be 59 changed files as it shows now. It is showing identical changes that have been merged already. I am thinking the number of changed files should be on the order of 10 or so -- basically only the ones you changed/deleted/added.

@neilsutcliffe
Copy link
Contributor Author

The styles bit will also conflict with #422, however that PR doesn't merge the style file for the generate.js file as well.

@neilsutcliffe
Copy link
Contributor Author

Feel free to use whatever you like from this, but I'm out. I have my own projects I've been putting off.

The main concerns I have are that English is considered the default language. i18n is the main distinction between this product and something like Jekyll, and good open source projects may come from anywhere across the globe.

If there the English translation is default, how would you get an English translation project that has Japanese as the "en" language.

Anyway, good luck.

@JoelMarcey
Copy link
Contributor

@neilsutcliffe will do. My plan was to use at least some of this as part of an overall v2.0 launch and a blog post.

I hope you weren’t putting off projects because of this particular PR. If so, my apologies. I appreciate the contributions you have provided to Docusaurus.

Good luck on your projects!

Sent with GitHawk

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

Successfully merging this pull request may close these issues.