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(v2): replace EJS with Eta for SSR generation #2684

Merged
merged 10 commits into from
May 5, 2020
Merged

refactor(v2): replace EJS with Eta for SSR generation #2684

merged 10 commits into from
May 5, 2020

Conversation

nebrelbug
Copy link
Contributor

Motivation

While working on some features for Docusaurus v2, I noticed that it uses EJS to generate SSR templates.

Eta is a template engine I created as an alternative to EJS, doT, and lodash.template. It is more lightweight, faster, and has more reliable parsing. It also exposes TypeScript types and collects test coverage, and keeps almost the same template syntax.

Because Eta supports caching intermediate template functions with the name field, avoids using with() {}, and has a faster parser than EJS anyway, I think it would lead to a slight increase in SSR rendering speed, as well as shave off a few KB from the user install size.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

  • Make sure the Netlify preview deployment works
  • Compare results of build folders

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Apr 27, 2020
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Apr 27, 2020

Deploy preview for docusaurus-2 ready!

Built with commit 672834e

https://deploy-preview-2684--docusaurus-2.netlify.app

@nebrelbug
Copy link
Contributor Author

nebrelbug commented Apr 27, 2020

Everything works on my local computer and the deploy previews 😃

@nebrelbug nebrelbug marked this pull request as ready for review April 27, 2020 21:29
Copy link
Contributor

@lex111 lex111 left a comment

Choose a reason for hiding this comment

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

@nebrelbug great, I would be happy to replace EJS and use your lib instead, but let's achieve the same result.
Can we remove the whitespaces as before? (on the right in the screenshot is Eta)

снимок_11

@nebrelbug
Copy link
Contributor Author

@lex111 sure! I actually ended up removing that because I saw the HTML is minified with https://github.com/DanielRuf/html-minifier-terser, but I can get it working again :)

@nebrelbug
Copy link
Contributor Author

nebrelbug commented Apr 28, 2020

@lex111 there are a few options:

  1. I set the equivalent of rmWhitespace on Eta (EJS' rmWhitespace calls templateText = templateText.replace(/[\r\n]+/g, '\n').replace(/^\s+|\s+$/gm, ''))
  2. I remove the whitespace in the template file

Which would you prefer?

@lex111
Copy link
Contributor

lex111 commented Apr 28, 2020

@nebrelbug let's use the equivalent of rmWhitespace option.

@nebrelbug
Copy link
Contributor Author

@lex111 just updated!

Doing the replace() up front in the template is clearest in my opinion, and allows Eta to only parse the template once 😃

Copy link
Contributor

@lex111 lex111 left a comment

Choose a reason for hiding this comment

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

Awesome, I definitely like this replacement!

image

@yangshun could you please take a look at this PR?

@lex111
Copy link
Contributor

lex111 commented Apr 29, 2020

@nebrelbug I'm just wondering why now (with Eta) and earlier (with EJS) so many whitespaces have not been deleted, as shown in the screenshot?

image

@nebrelbug
Copy link
Contributor Author

@lex111 I think specifically those whitespaces are from values Docusaurus passed as data to the template engine -- not from the template. Neither Eta nor EJS removes whitespace from user data. That's just a guess though -- I'll have to look into it more

@lex111
Copy link
Contributor

lex111 commented Apr 29, 2020

@nebrelbug yep, you're right, but how to remove them, is there an easy and safe way to do this?https://github.com/facebook/docusaurus/blob/master/website/docusaurus.config.js#L186

@nebrelbug
Copy link
Contributor Author

@lex111 sure! Just remove the whitespace in the template literal.

              html: `
                <a href="https://www.netlify.com" target="_blank" rel="noreferrer noopener" aria-label="Deploys by Netlify">
                  <img src="https://www.netlify.com/img/global/badges/netlify-color-accent.svg" alt="Deploys by Netlify" />
                </a>
              `,

Can be changed to:

              html: `
<a href="https://www.netlify.com" target="_blank" rel="noreferrer noopener" aria-label="Deploys by Netlify">
  <img src="https://www.netlify.com/img/global/badges/netlify-color-accent.svg" alt="Deploys by Netlify" />
</a>
`,

@lex111
Copy link
Contributor

lex111 commented Apr 29, 2020 via email

@nebrelbug
Copy link
Contributor Author

@lex111 we could change:

<%~ it.postBodyTags %>

To:

<%~ it.postBodyTags.replace(/^\s+/gm, '') %>

(I'm assuming the Netlify button is in postBodyTags?)

@nebrelbug
Copy link
Contributor Author

@lex111 or we could define a custom helper function or write a plugin or do a few other things. There isn't a built-in way to do this though in either Eta or EJS

@lex111
Copy link
Contributor

lex111 commented Apr 30, 2020

@nebrelbug no problem, this is completely optional, I was curious how this could be done. So Eta seems like a good replacement for me!

Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

Fingers crossed 💣

@yangshun yangshun merged commit 368eb42 into facebook:master May 5, 2020
@yangshun yangshun added pr: maintenance This PR does not produce any behavior differences to end users when upgrading. pr: performance This PR does not add a new behavior, but existing behaviors will be more memory- / time-efficient. and removed pr: maintenance This PR does not produce any behavior differences to end users when upgrading. labels May 5, 2020
@lex111 lex111 added this to the v2.0.0-alpha.55 milestone May 19, 2020
@nebrelbug
Copy link
Contributor Author

@lex111 and @yangshun:

By the way, may I list Docusaurus 2 under Eta's README section called "Projects using Eta"?

@yangshun
Copy link
Contributor

yangshun commented Jul 4, 2020

@nebrelbug Of course! Happy for you to do that!

@yangshun
Copy link
Contributor

yangshun commented Jul 4, 2020

We would love for you to showcase Eta on https://v2.docusaurus.io/showcase as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: performance This PR does not add a new behavior, but existing behaviors will be more memory- / time-efficient.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants