Skip to content

Commit

Permalink
Merge pull request #1411 from sharetribe/code-splitting-dataloader-api
Browse files Browse the repository at this point in the history
Allow code splitting
  • Loading branch information
Gnito authored Feb 17, 2021
2 parents 10661cc + ecfa9e7 commit 09a1815
Show file tree
Hide file tree
Showing 74 changed files with 1,404 additions and 468 deletions.
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ public/
src/components/index.js
src/containers/index.js
src/forms/index.js
src/routeConfiguration.js
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ way to update this template, but currently, we follow a pattern:

## Upcoming version 2020-XX-XX

- [add] Route-based code splitting. This is done against sharetribe-scripts v5.0.0 using Loadable
components. Read more from the pull request.
[#1411](https://github.com/sharetribe/ftw-daily/pull/1411)

## [v7.3.0] 2021-01-13

- [fix] Move well-known/\* endpoints related to OIDC proxy setup from `apiRouter` to new
Expand Down
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
"dependencies": {
"@babel/runtime": "^7.11.2",
"@formatjs/intl-relativetimeformat": "^4.2.1",
"@loadable/component": "^5.14.1",
"@loadable/server": "^5.14.2",
"@mapbox/polyline": "^1.1.1",
"@sentry/browser": "5.20.1",
"@sentry/node": "5.20.1",
Expand Down Expand Up @@ -58,7 +60,7 @@
"redux-thunk": "^2.3.0",
"seedrandom": "^3.0.5",
"sharetribe-flex-sdk": "1.13.0",
"sharetribe-scripts": "4.0.0",
"sharetribe-scripts": "5.0.0",
"smoothscroll-polyfill": "^0.4.0",
"source-map-support": "^0.5.9",
"url": "^0.11.0"
Expand Down Expand Up @@ -93,7 +95,9 @@
"dev-frontend": "sharetribe-scripts start",
"dev-backend": "nodemon server/apiServer.js",
"dev": "yarn run config-check&&export NODE_ENV=development REACT_APP_DEV_API_SERVER_PORT=3500&&concurrently --kill-others \"yarn run dev-frontend\" \"yarn run dev-backend\"",
"build": "sharetribe-scripts build",
"build": "yarn build-web&&yarn build-server",
"build-web": "sharetribe-scripts build",
"build-server": "sharetribe-scripts build-server",
"format": "prettier --write '**/*.{js,css}'",
"format-ci": "prettier --list-different '**/*.{js,css}'",
"format-docs": "prettier --write '**/*.md'",
Expand Down
4 changes: 4 additions & 0 deletions public/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
<!-- End Favicons -->

<!--!link-->
<!--!ssrStyles-->
<!--!ssrLinks-->

<style>
/**
Expand Down Expand Up @@ -161,6 +163,8 @@
<!--!preloadedStateScript-->
<!--!script-->

<!--!ssrScripts-->

<!--
Note when adding new external scripts/styles/fonts/etc.:
If a Content Security Policy (CSP) is turned on, the new URLs
Expand Down
3 changes: 1 addition & 2 deletions server/dataLoader.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
const url = require('url');
const { matchPathname, configureStore, routeConfiguration } = require('./importer');
const log = require('./log');

exports.loadData = function(requestUrl, sdk) {
exports.loadData = function(requestUrl, sdk, matchPathname, configureStore, routeConfiguration) {
const { pathname, query } = url.parse(requestUrl);
const matchedRoutes = matchPathname(pathname, routeConfiguration());

Expand Down
24 changes: 16 additions & 8 deletions server/importer.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,25 @@
*/

const path = require('path');
const { ChunkExtractor } = require('@loadable/server');

// Construct the bundle path where the bundle exports can be imported
const buildPath = path.resolve(__dirname, '..', 'build');
const manifestPath = path.join(buildPath, 'asset-manifest.json');
const manifest = require(manifestPath);
const mainJsPath = path.join(buildPath, manifest['files']['main.js']);
const mainJs = require(mainJsPath);

// lodabale-stats.json files from node and web builds
const nodeStats = path.join(buildPath, 'node/loadable-stats.json');
const webStats = path.join(buildPath, 'loadable-stats.json');

module.exports = {
renderApp: mainJs.default,
matchPathname: mainJs.matchPathname,
configureStore: mainJs.configureStore,
routeConfiguration: mainJs.routeConfiguration,
getExtractors: () => {
const nodeExtractor = new ChunkExtractor({
statsFile: nodeStats,
outputPath: path.resolve(__dirname, '..', 'build/node'),
});
const webExtractor = new ChunkExtractor({
statsFile: webStats,
outputPath: path.resolve(__dirname, '..', 'build'),
});
return { nodeExtractor, webExtractor };
},
};
13 changes: 11 additions & 2 deletions server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const passport = require('passport');
const auth = require('./auth');
const apiRouter = require('./apiRouter');
const wellKnownRouter = require('./wellKnownRouter');
const { getExtractors } = require('./importer');
const renderer = require('./renderer');
const dataLoader = require('./dataLoader');
const fs = require('fs');
Expand Down Expand Up @@ -214,10 +215,18 @@ app.get('*', (req, res) => {
// data, let's disable response caching altogether.
res.set(noCacheHeaders);

// Get chunk extractors from node and web builds
// https://loadable-components.com/docs/api-loadable-server/#chunkextractor
const { nodeExtractor, webExtractor } = getExtractors();

// Server-side entrypoint provides us the functions for server-side data loading and rendering
const nodeEntrypoint = nodeExtractor.requireEntrypoint();
const { default: renderApp, matchPathname, configureStore, routeConfiguration } = nodeEntrypoint;

dataLoader
.loadData(req.url, sdk)
.loadData(req.url, sdk, matchPathname, configureStore, routeConfiguration)
.then(preloadedState => {
const html = renderer.render(req.url, context, preloadedState);
const html = renderer.render(req.url, context, preloadedState, renderApp, webExtractor);

if (dev) {
const debugData = {
Expand Down
11 changes: 8 additions & 3 deletions server/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const path = require('path');
const fs = require('fs');
const _ = require('lodash');
const { types } = require('sharetribe-flex-sdk');
const { renderApp } = require('./importer');

const buildPath = path.resolve(__dirname, '..', 'build');

Expand Down Expand Up @@ -91,8 +90,11 @@ const replacer = (key = null, value) => {
return types.replacer(key, cleanedValue);
};

exports.render = function(requestUrl, context, preloadedState) {
const { head, body } = renderApp(requestUrl, context, preloadedState);
exports.render = function(requestUrl, context, preloadedState, renderApp, webExtractor) {
// Bind webExtractor as "this" for collectChunks call.
const collectWebChunks = webExtractor.collectChunks.bind(webExtractor);

const { head, body } = renderApp(requestUrl, context, preloadedState, collectWebChunks);

// Preloaded state needs to be passed for client side too.
// For security reasons we ensure that preloaded state is considered as a string
Expand Down Expand Up @@ -133,6 +135,9 @@ exports.render = function(requestUrl, context, preloadedState) {
script: head.script.toString(),
preloadedStateScript,
googleAnalyticsScript,
ssrStyles: webExtractor.getStyleTags(),
ssrLinks: webExtractor.getLinkTags(),
ssrScripts: webExtractor.getScriptTags(),
body,
});
};
4 changes: 2 additions & 2 deletions src/Routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,13 @@ class RouteComponentRenderer extends Component {

render() {
const { route, match, location, staticContext } = this.props;
const { component: RouteComponent, authPage = 'SignupPage' } = route;
const { component: RouteComponent, authPage = 'SignupPage', extraProps } = route;
const canShow = canShowComponent(this.props);
if (!canShow) {
staticContext.unauthorized = true;
}
return canShow ? (
<RouteComponent params={match.params} location={location} />
<RouteComponent params={match.params} location={location} {...extraProps} />
) : (
<NamedRedirect
name={authPage}
Expand Down
8 changes: 6 additions & 2 deletions src/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,17 +131,21 @@ ServerApp.propTypes = { url: string.isRequired, context: any.isRequired, store:
* - {String} body: Rendered application body of the given route
* - {Object} head: Application head metadata from react-helmet
*/
export const renderApp = (url, serverContext, preloadedState) => {
export const renderApp = (url, serverContext, preloadedState, collectChunks) => {
// Don't pass an SDK instance since we're only rendering the
// component tree with the preloaded store state and components
// shouldn't do any SDK calls in the (server) rendering lifecycle.
const store = configureStore(preloadedState);

const helmetContext = {};

const body = ReactDOMServer.renderToString(
// When rendering the app on server, we wrap the app with webExtractor.collectChunks
// This is needed to figure out correct chunks/scripts to be included to server-rendered page.
// https://loadable-components.com/docs/server-side-rendering/#3-setup-chunkextractor-server-side
const WithChunks = collectChunks(
<ServerApp url={url} context={serverContext} helmetContext={helmetContext} store={store} />
);
const body = ReactDOMServer.renderToString(WithChunks);
const { helmet: head } = helmetContext;
return { head, body };
};
24 changes: 0 additions & 24 deletions src/app.node.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,30 +32,6 @@ describe('Application - node environment', () => {
render('/styleguide', {});
});

it('server renders pages that do not require authentication', () => {
const urlTitles = {
'/': 'LandingPage.schemaTitle',
'/s': 'SearchPage.schemaTitle',
'/l/listing-title-slug/1234': 'ListingPage.loadingListingTitle',
'/l/1234': 'ListingPage.loadingListingTitle',
'/u/1234': 'ProfilePage.schemaTitle',
'/login': 'AuthenticationPage.schemaTitleLogin',
'/signup': 'AuthenticationPage.schemaTitleSignup',
'/recover-password': 'PasswordRecoveryPage.title',
'/this-url-should-not-be-found': 'NotFoundPage.title',
'/reset-password?t=token&e=email': 'PasswordResetPage.title',
};
forEach(urlTitles, (title, url) => {
const context = {};
const { head, body } = render(url, context);

expect(head.title.toString()).toContain(title);

// context.url will contain the URL to redirect to if a <Redirect> was used
expect(context.url).not.toBeDefined();
});
});

it('server renders redirects for pages that require authentication', () => {
const loginPath = '/login';
const signupPath = '/signup';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ exports[`ActivityFeed matches snapshot 1`] = `
className="root avatar"
href="/u/user1"
onClick={[Function]}
onMouseOver={[Function]}
onTouchStart={[Function]}
style={Object {}}
title="user1 display name"
>
Expand Down
28 changes: 24 additions & 4 deletions src/components/Button/Button.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { bool, node, string } from 'prop-types';
import classNames from 'classnames';
import routeConfiguration from '../../routeConfiguration';
import { findRouteByRouteName } from '../../util/routes';
import { IconSpinner, IconCheckmark } from '../../components';

import css from './Button.module.css';
Expand All @@ -23,6 +25,7 @@ class Button extends Component {
inProgress,
ready,
disabled,
enforcePagePreloadFor,
...rest
} = this.props;

Expand All @@ -42,21 +45,36 @@ class Button extends Component {
content = children;
}

const onOverButtonFn = enforcePreloadOfPage => () => {
// Enforce preloading of given page (loadable component)
const { component: Page } = findRouteByRouteName(enforcePreloadOfPage, routeConfiguration());
// Loadable Component has a "preload" function.
if (Page.preload) {
Page.preload();
}
};

const onOverButton = enforcePagePreloadFor ? onOverButtonFn(enforcePagePreloadFor) : null;
const onOverButtonMaybe = onOverButton
? {
onMouseOver: onOverButton,
onTouchStart: onOverButton,
}
: {};

// All buttons are disabled until the component is mounted. This
// prevents e.g. being able to submit forms to the backend before
// the client side is handling the submit.
const buttonDisabled = this.state.mounted ? disabled : true;

return (
<button className={classes} {...rest} disabled={buttonDisabled}>
<button className={classes} {...onOverButtonMaybe} {...rest} disabled={buttonDisabled}>
{content}
</button>
);
}
}

const { node, string, bool } = PropTypes;

Button.defaultProps = {
rootClassName: null,
className: null,
Expand All @@ -65,6 +83,7 @@ Button.defaultProps = {
inProgress: false,
ready: false,
disabled: false,
enforcePagePreloadFor: null,
children: null,
};

Expand All @@ -77,6 +96,7 @@ Button.propTypes = {
inProgress: bool,
ready: bool,
disabled: bool,
enforcePagePreloadFor: string,

children: node,
};
Expand Down
29 changes: 25 additions & 4 deletions src/components/Form/Form.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,27 @@
import React from 'react';
import PropTypes from 'prop-types';
import { func, node, string } from 'prop-types';
import routeConfiguration from '../../routeConfiguration';
import { findRouteByRouteName } from '../../util/routes';

const Form = props => {
const { children, contentRef, ...restProps } = props;
const { children, contentRef, enforcePagePreloadFor, ...restProps } = props;

const onOverFormFn = enforcePreloadOfPage => () => {
// Enforce preloading of given page (loadable component)
const { component: Page } = findRouteByRouteName(enforcePreloadOfPage, routeConfiguration());
// Loadable Component has a "preload" function.
if (Page.preload) {
Page.preload();
}
};

const onOverForm = enforcePagePreloadFor ? onOverFormFn(enforcePagePreloadFor) : null;
const onOverFormMaybe = onOverForm
? {
onMouseOver: onOverForm,
onTouchStart: onOverForm,
}
: {};

const formProps = {
// These are mainly default values for the server
Expand All @@ -15,21 +34,23 @@ const Form = props => {
// allow content ref function to be passed to the form
ref: contentRef,

...onOverFormMaybe,
...restProps,
};

return <form {...formProps}>{children}</form>;
};

Form.defaultProps = {
children: null,
contentRef: null,
enforcePagePreloadFor: null,
};

const { func, node } = PropTypes;

Form.propTypes = {
children: node,
contentRef: func,
enforcePagePreloadFor: string,
};

export default Form;
Loading

0 comments on commit 09a1815

Please sign in to comment.