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

feat(v2): implement ContentRenderer #1366

Merged
merged 7 commits into from
Apr 16, 2019
Merged

feat(v2): implement ContentRenderer #1366

merged 7 commits into from
Apr 16, 2019

Conversation

yangshun
Copy link
Contributor

Motivation

Add a ContentRenderer component that will help a component fetch the modules it needs based on the path provided.

Approach

  • Generate two files: registry.js and routesAsyncModules.json.
    • registry.js is simply a mapping of chunk names to their import statements.
    • routesAsyncModules.json is a mapping of routes to the chunks it needs, with the added benefit of having a shape
  • How ContentRenderer works:
    • In each component, (e.g. BlogPost or BlogPage), pass ContentRenderer the current path.
    • Look up routesAsyncModules with that path to get the chunks that are needed
    • Use React Loadable to load the chunks in parallel and pass them back to the component.

End Result

  • Cleaner and smaller routes file. The route files no longer have the import statements for modules. TODO: Make metadata use a similar approach. Eventually the routes file will be very clean and lean, as we're de-compiling the code and providing the dependent modules to React Loadable during runtime. Eventually for each route it will just be as simple as below, without any of the metadata/modules stuff.
{
  path: '/blog/2017/12/14/introducing-docusaurus',
  exact: true,
  component: Loadable({
    loader: () => import('./components/Bar'),
    loading: Loading,
  }),
}

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Netlify.

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@yangshun yangshun requested a review from endiliey as a code owner April 15, 2019 06:32
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Apr 15, 2019
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Apr 15, 2019

Deploy preview for docusaurus-2 ready!

Built with commit b26f8df

https://deploy-preview-1366--docusaurus-2.netlify.com

@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Apr 15, 2019

Deploy preview for docusaurus-preview ready!

Built with commit b26f8df

https://deploy-preview-1366--docusaurus-preview.netlify.com

Copy link
Contributor

@endiliey endiliey left a comment

Choose a reason for hiding this comment

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

Unfortunately this broke the SSR because we're making a Loadable of Loadable.

If you go to https://deploy-preview-1366--docusaurus-2.netlify.com/blog/
or https://deploy-preview-1366--docusaurus-2.netlify.com/blog/2017/12/14/introducing-docusaurus/

The generated html file is also empty
image

Actually what it did is rendering the Loading screen in the static html file

If we change default-theme/Loading/index.js

export default props => {
  if (props.error) {
    console.log(props.error);
    return <div align="center">Error</div>;
  }

  if (props.pastDelay) {
    return (
      <div className={styles.loader}>
        <p>Please wait a moment</p>
        <div className={styles.loaderSpinning} />
      </div>
    );
  }

  // return null; Old code
  return 'This is a loading screen';
};

The HTML is actually a loading screen
image

If we go to https://deploy-preview-1366--docusaurus-2.netlify.com/blog/2017/12/14/introducing-docusaurus/

The error is not particularly useful,
image

I tried changing our webpack config devtool to be devtool: 'eval-source-map',
image

I suspect this is because we are initializing Loadable in render() when it shouldn't

image

I am not sure how to resolve this 😢

@endiliey

This comment has been minimized.

* wip

* avoid chunk names collision

* ContentRenderer is a wrapper for Loadable

* convert docs and pages

* nits and rename

* rename routeModules -> modules

* remove lodash from component creator

* resolve chunk not being picked up correctly

* add comment for explanation
@endiliey endiliey merged commit 96cb467 into master Apr 16, 2019
@endiliey endiliey deleted the content-renderer branch April 16, 2019 14:18
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.

4 participants