Defer babel loading #100

renanpvaz · 2017-10-09T16:42:20Z

Created an utility function to asynchronously load Babel and Algolia attempting to fix #2.

facebook-github-bot · 2017-10-09T16:42:28Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

reactjs-bot · 2017-10-09T16:45:58Z

Deploy preview ready!

Built with commit ba6a786

https://deploy-preview-100--reactjs.netlify.com

facebook-github-bot · 2017-10-09T16:48:31Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

bvaughn · 2017-10-09T20:22:50Z

src/layouts/index.js

+        apiKey: '36221914cce388c46d0420343e0bb32e',
+        indexName: 'react',
+        inputSelector: '#algolia-doc-search',
+      });


This will load the Algolia JS for every page. Even though the request will be cached, the browser might still end up wasting cycles processing/reprocessing the script.

Also, ideally, if we're going to defer loading Algolia- we should defer loading it until the user actually starts to search (eg clicks to focus in the search bar). So maybe we could do this async step there instead? Happy to (a) talk more about this or (b) split that part off into a separate, follow-up issue or task.

Perhaps this should be a separate issue.

bvaughn · 2017-10-09T20:23:34Z

src/templates/home.js

+      mountCodeExample('timerExample', TIMER_COMPONENT);
+      mountCodeExample('todoExample', TODO_COMPONENT);
+      mountCodeExample('markdownExample', MARKDOWN_COMPONENT);
+    });


What are your thoughts about showing a loading placeholder in the 4 divs in question until Babel has finished loading?

It's a good idea, I can see that it is flashing once it loads. To fix this should I style the divs at content/index.md?

Probably not. (We actually need to clean up index.md, see #97)

I would try maybe temporarily rendering a placeholder into the container from the Home template.

So you mean that I should use ReactDOM.render like mountCodeExample does?

Probably 😄

…defer-babel-loading

bvaughn

Noticed 2 small things. Sorry if you're still working on these files and my comments are premature.

bvaughn · 2017-10-10T21:58:10Z

src/layouts/index.js

@@ -19,6 +19,8 @@ import Flex from 'components/Flex';
 import Footer from 'components/LayoutFooter';
 import Header from 'components/LayoutHeader';
 import {media} from 'theme';
+import loadScript from 'utils/loadScript';
+import {algoliaURL} from 'site-constants';


These imports can be removed ^

bvaughn · 2017-10-10T21:59:04Z

src/site-constants.js


-export {urlRoot, version};
+export {urlRoot, version, babelURL,};


Run yarn prettier to fix this formatting ^ (the trailing comma should be removed)

bvaughn

I'm curious about all of these changes to CodeEditor markup/styles. Why are they needed for deferred Babel loading?

I'm a bit reluctant to change this markup much since we've fixed a lot of edge-cases with scrolling and responsive behavior for this specific component. Is there a way we could do the deferred loading that didn't require so many changes?

I had envisioned something like this:

When the page initially mounts, render some placeholder "Loading code example..." type string into the 4 container <div>s
Lazy-load Babel (like this PR does)
Then mount the CodeEditor component and compiled results into the 4 container <div>s, replacing the placeholder content.

renanpvaz · 2017-10-11T15:46:03Z

Well, most of the changes are not related to Babel, I was having trouble trying to render the placeholders because of the way that mountCodeExample works and the html structure of the examples. Trying not to touch index.md and thinking that you meant a more elaborate placeholder, I decided to remove all the specific styling that CodeEditor had for children and instead did something similar inside mountCodeExample. Besides that, one of the things I was trying to solve is the "raw" appearance (with the backticks) of the examples before mountCodeExample is called.

Anyways, if you're unsure about the changes I could do what you envisioned, but I think that the transition would still look rough once Babel loads.

bvaughn · 2017-10-11T16:07:54Z

Thanks for explaining. 😄

I'd like to avoid making any changes (at least bigs ones) to CodeEditor for the time being. We've done a lot of iterating on that component to get it to look and work right.

We don't have to use the mountCodeExample util to set the placeholder. It's just a convenience wrapper for code that is controlled by the CodeEditor. Our placeholders could just be mounted with ReactDOM.render(...)

Let's do with a simple placeholder right now (just a string is fine) and myself and/or @joecritch will follow up on it.

…defer-babel-loading

bvaughn · 2017-10-11T17:57:41Z

Apologies for forgetting about the hackiness we were doing inside of mountCodeExample. Now I understand why you took the original approach you did with this PR.

The home page content and markdown is all pretty janky until #97 gets resolved unfortunately, so
it's probably okay for now to just live with a less-than-perfect solution.

Ideally we'd show a nice placeholder when loading is in progress and a nice error if loading failed. When I say "nice" I don't mean "fancy" just- something that will prevent the page from re-flowing text drastically when things load in. Unfortunately this isn't possible without more hacking b'c of how our content is put together and what we're doing in mountCodeExample.

How do you feel about (for now) further simplifying this to:

diff --git a/src/templates/home.js b/src/templates/home.js
index 5131557e3..cb32c683c 100644
--- a/src/templates/home.js
+++ b/src/templates/home.js
@@ -21,10 +21,10 @@ import ReactDOM from 'react-dom';
 
 class Home extends Component {
   componentDidMount() {
-    this.renderExamplePlaceholder('helloExample');
-    this.renderExamplePlaceholder('timerExample');
-    this.renderExamplePlaceholder('todoExample');
-    this.renderExamplePlaceholder('markdownExample');
+    renderExamplePlaceholder('helloExample');
+    renderExamplePlaceholder('timerExample');
+    renderExamplePlaceholder('todoExample');
+    renderExamplePlaceholder('markdownExample');
 
     loadScript(babelURL).then(() => {
       mountCodeExample('helloExample', HELLO_COMPONENT);
@@ -34,25 +34,6 @@ class Home extends Component {
     });
   }
 
-  renderExamplePlaceholder(containerId) {
-    ReactDOM.render(
-      <strong
-        css={{
-          transform: 'translate(65%, -5em)',
-          display: 'block',
-
-          [media.lessThan('xlarge')]: {
-            transform: 'none',
-            marginTop: 15,
-            textAlign: 'center',
-          },
-        }}>
-        Loading code example...
-      </strong>,
-      document.getElementById(containerId),
-    );
-  }
-
   render() {
     const {data} = this.props;
     const title = 'React - A JavaScript library for building user interfaces';
@@ -341,6 +322,16 @@ const markdownStyles = {
   },
 };
 
+// TODO Improve this temporarily placeholder as part of 
+// converting the home page from markdown template to a Gatsby
+// page (see issue #2)
+function renderExamplePlaceholder(containerId) {
+  ReactDOM.render(
+    <h4>Loading code example...</h4>,
+    document.getElementById(containerId),
+  );
+}
+
 // TODO Move these hard-coded examples into example files and out of the template?
 // Alternately, move them into the markdown and transform them during build?
 // This could be done via a new Gatsby transform plug-in that auto-converts to runnable REPLs?

The reasons I propose these changes are:

Moving the placeholder text up via a negative offset seemed kind of hacky and caused inconsistent vertical alignment. Since text reflows anyway when the code loads in (at least currently) the hack wasn't buying us much anyway.
nit: There's no need for the util method to be an instance method since it doesn't access anything on the instance

renanpvaz · 2017-10-11T18:37:39Z

Sure thing! Oh, and thanks for all the patience since this is my first PR with actual code.

bvaughn · 2017-10-11T18:39:18Z

No thanks needed! Thanks for contributing and being so easy to work with and iterate on this stuff 😄

bvaughn

I dig it

* docs: Translation of Forms page * docs: Small corrections in "Forms" page Co-Authored-By: Halian Vilela <halian-vilela@users.noreply.github.com> * docs: Removing accent from an example on the Forms page Co-Authored-By: glaucia86 <glaucia_lemos86@hotmail.com>

…hinese (reactjs#100) translate content/docs/reference-react-dom-server.md into Chinese

renanpvaz added 3 commits October 9, 2017 13:12

load babel only when needed

1d48f75

linting fixes

80b1b83

update constant name

7efe10f

facebook-github-bot added the CLA Signed label Oct 9, 2017

bvaughn requested changes Oct 9, 2017

View reviewed changes

renanpvaz added 2 commits October 9, 2017 17:38

Merge branch 'master' of https://github.com/reactjs/reactjs.org into …

b843de7

…defer-babel-loading

undo changes to algolia search

a02139b

bvaughn reviewed Oct 10, 2017

View reviewed changes

bvaughn requested changes Oct 11, 2017

View reviewed changes

renanpvaz force-pushed the defer-babel-loading branch from 49cf1f7 to a02139b Compare October 11, 2017 16:16

renanpvaz added 2 commits October 11, 2017 13:59

render simple placeholder while Babel loads

36e1185

Merge branch 'master' of https://github.com/reactjs/reactjs.org into …

0a255fe

…defer-babel-loading

bvaughn mentioned this pull request Oct 11, 2017

Remove hard-coded HTML/CSS from index.md #97

Closed

simplify placeholder

ba6a786

bvaughn approved these changes Oct 11, 2017

View reviewed changes

bvaughn merged commit 85a70b7 into reactjs:master Oct 11, 2017

BetterZxx pushed a commit to BetterZxx/react.dev that referenced this pull request Mar 21, 2023

docs(cn): translate content/docs/reference-react-dom-server.md into C…

9eff452

…hinese (reactjs#100) translate content/docs/reference-react-dom-server.md into Chinese

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Defer babel loading #100

Defer babel loading #100

renanpvaz commented Oct 9, 2017

facebook-github-bot commented Oct 9, 2017

reactjs-bot commented Oct 9, 2017 •

edited

facebook-github-bot commented Oct 9, 2017

bvaughn Oct 9, 2017

renanpvaz Oct 10, 2017

bvaughn Oct 9, 2017

renanpvaz Oct 9, 2017

bvaughn Oct 9, 2017

renanpvaz Oct 9, 2017

bvaughn Oct 9, 2017

bvaughn left a comment

bvaughn Oct 10, 2017

bvaughn Oct 10, 2017

bvaughn left a comment

renanpvaz commented Oct 11, 2017

bvaughn commented Oct 11, 2017 •

edited

bvaughn commented Oct 11, 2017 •

edited

renanpvaz commented Oct 11, 2017

bvaughn commented Oct 11, 2017

bvaughn left a comment


		export {urlRoot, version};
		export {urlRoot, version, babelURL,};

Defer babel loading #100

Defer babel loading #100

Conversation

renanpvaz commented Oct 9, 2017

facebook-github-bot commented Oct 9, 2017

reactjs-bot commented Oct 9, 2017 • edited

facebook-github-bot commented Oct 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvaughn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bvaughn left a comment

Choose a reason for hiding this comment

renanpvaz commented Oct 11, 2017

bvaughn commented Oct 11, 2017 • edited

bvaughn commented Oct 11, 2017 • edited

renanpvaz commented Oct 11, 2017

bvaughn commented Oct 11, 2017

bvaughn left a comment

Choose a reason for hiding this comment

reactjs-bot commented Oct 9, 2017 •

edited

bvaughn commented Oct 11, 2017 •

edited

bvaughn commented Oct 11, 2017 •

edited