Skip to content

Commit

Permalink
Remove code-splitting feature (#10093)
Browse files Browse the repository at this point in the history
  • Loading branch information
willdurand committed Feb 16, 2021
1 parent 54b8f81 commit 5f08286
Show file tree
Hide file tree
Showing 20 changed files with 29 additions and 467 deletions.
4 changes: 1 addition & 3 deletions .babelrc
Expand Up @@ -11,9 +11,7 @@
"@babel/preset-react"
],
"plugins": [
"@babel/plugin-proposal-class-properties",
"@babel/plugin-syntax-dynamic-import",
"@loadable/babel-plugin"
"@babel/plugin-proposal-class-properties"
],
"env": {
"test": {
Expand Down
11 changes: 1 addition & 10 deletions config/default.js
Expand Up @@ -8,16 +8,10 @@ import { addonsServerProdCDN, analyticsHost, prodDomain, apiProdHost, baseUrlPro

const addonsFrontendCDN = 'https://addons-amo.cdn.mozilla.net';
const basePath = path.resolve(__dirname, '../');
const distPath = path.join(basePath, 'dist');
const loadableStatsFilename = 'loadable-stats.json';

module.exports = {
basePath,

// This is needed for code-splitting.
loadableStatsFilename,
loadableStatsFile: path.join(distPath, loadableStatsFilename),

// The base URL of the site (for SEO purpose).
baseURL: baseUrlProd,

Expand Down Expand Up @@ -142,6 +136,7 @@ module.exports = {
// from ./lib/shared.js
CSP: {
directives: {
defaultSrc: ["'none'"],
baseUri: ["'self'"],
childSrc: ["'none'"],
connectSrc: [analyticsHost, apiProdHost, sentryHost],
Expand All @@ -157,10 +152,6 @@ module.exports = {
manifestSrc: ["'none'"],
mediaSrc: ["'none'"],
objectSrc: ["'none'"],
// This is needed because `prefetchSrc` isn't supported by FF yet.
// See: https://bugzilla.mozilla.org/show_bug.cgi?id=1457204
defaultSrc: [addonsFrontendCDN],
prefetchSrc: [addonsFrontendCDN],
// Script is limited to the amo specific CDN.
scriptSrc: [
addonsFrontendCDN,
Expand Down
4 changes: 0 additions & 4 deletions config/dev.js
Expand Up @@ -37,10 +37,6 @@ module.exports = {
styleSrc: [
addonsFrontendCDN,
],
// This is needed because `prefetchSrc` isn't supported by FF yet.
// See: https://bugzilla.mozilla.org/show_bug.cgi?id=1457204
defaultSrc: [addonsFrontendCDN],
prefetchSrc: [addonsFrontendCDN],
},
},

Expand Down
4 changes: 0 additions & 4 deletions config/development.js
Expand Up @@ -77,10 +77,6 @@ module.exports = {
// webpack injects inline CSS
"'unsafe-inline'",
],
// This is needed because `prefetchSrc` isn't supported by FF yet.
// See: https://bugzilla.mozilla.org/show_bug.cgi?id=1457204
defaultSrc: [webpackHost],
prefetchSrc: [webpackHost],
},
reportOnly: true,
},
Expand Down
4 changes: 0 additions & 4 deletions config/stage.js
Expand Up @@ -35,10 +35,6 @@ module.exports = {
styleSrc: [
addonsFrontendCDN,
],
// This is needed because `prefetchSrc` isn't supported by FF yet.
// See: https://bugzilla.mozilla.org/show_bug.cgi?id=1457204
defaultSrc: [addonsFrontendCDN],
prefetchSrc: [addonsFrontendCDN],
},
},

Expand Down
7 changes: 0 additions & 7 deletions config/test.js
@@ -1,7 +1,3 @@
import path from 'path';

const fixturesPath = path.join(__dirname, '..', 'tests', '__fixtures__');

// Put any test configuration overrides here.
module.exports = {
// No test should touch the API so seeing this would indicate a bug.
Expand All @@ -16,8 +12,5 @@ module.exports = {
// Force-disable Sentry
publicSentryDsn: null,

// We use a fake/incomplete file for the test suite.
loadableStatsFile: path.join(fixturesPath, 'loadable-stats.json'),

mozillaUserId: 1337,
};
5 changes: 0 additions & 5 deletions package.json
Expand Up @@ -173,8 +173,6 @@
},
"homepage": "https://github.com/mozilla/addons-frontend#readme",
"dependencies": {
"@loadable/component": "5.14.1",
"@loadable/server": "5.14.0",
"@mozilla-protocol/tokens": "5.0.5",
"@willdurand/isomorphic-formdata": "1.2.0",
"base62": "2.0.1",
Expand Down Expand Up @@ -258,14 +256,11 @@
"devDependencies": {
"@babel/core": "^7.1.6",
"@babel/plugin-proposal-class-properties": "^7.1.0",
"@babel/plugin-syntax-dynamic-import": "^7.0.0",
"@babel/preset-env": "^7.1.6",
"@babel/preset-flow": "^7.0.0",
"@babel/preset-react": "^7.0.0",
"@babel/register": "^7.0.0",
"@emotion/core": "^11.0.0",
"@loadable/babel-plugin": "^5.7.1",
"@loadable/webpack-plugin": "^5.2.1",
"@storybook/addon-docs": "^6.0.21",
"@storybook/addons": "^6.0.0",
"@storybook/react": "^6.0.0",
Expand Down
4 changes: 0 additions & 4 deletions renovate.json
Expand Up @@ -26,10 +26,6 @@
],
"groupName": "reactivestack/cookies"
},
{
"packagePatterns": ["@loadable/*"],
"groupName": "loadable components"
},
{
"packagePatterns": [
"history",
Expand Down
15 changes: 6 additions & 9 deletions src/amo/client/base.js
Expand Up @@ -7,7 +7,6 @@ import { createBrowserHistory } from 'history';
import RavenJs from 'raven-js';
import * as React from 'react';
import { render } from 'react-dom';
import { loadableReady } from '@loadable/component';

import Root from 'amo/components/Root';
import { langToLocale, makeI18n, sanitizeLanguage } from 'amo/i18n/utils';
Expand Down Expand Up @@ -123,14 +122,12 @@ export default async function createClient(
const i18n = makeI18n(i18nData, lang);

const renderApp = (App) => {
return loadableReady(() => {
render(
<Root history={history} i18n={i18n} store={store}>
<App />
</Root>,
document.getElementById('react-view'),
);
});
render(
<Root history={history} i18n={i18n} store={store}>
<App />
</Root>,
document.getElementById('react-view'),
);
};

return { history, renderApp, store };
Expand Down
26 changes: 2 additions & 24 deletions src/amo/components/Routes/index.js
Expand Up @@ -2,7 +2,6 @@
import config from 'config';
import * as React from 'react';
import { Route, Switch } from 'react-router-dom';
import loadable from '@loadable/component';

import Addon from 'amo/pages/Addon';
import AddonInfo, {
Expand Down Expand Up @@ -33,31 +32,10 @@ import UsersUnsubscribe from 'amo/pages/UsersUnsubscribe';
import SimulateAsyncError from 'amo/pages/error-simulation/SimulateAsyncError';
import SimulateClientError from 'amo/pages/error-simulation/SimulateClientError';
import SimulateSyncError from 'amo/pages/error-simulation/SimulateSyncError';
import About from 'amo/pages/StaticPages/About';
import ReviewGuide from 'amo/pages/StaticPages/ReviewGuide';
import type { ConfigType } from 'amo/types/config';

// About `loadable()` and code-splitting:
//
// 1. Set `webpackChunkName` to the name of the page component
// 2. Set `webpackPreload: true` as already done below
//
// Important: We do not use `webpackPrefetch: true` to prevent webpack to
// inject anything in the HTML returned by the server. Webpack does not inject
// anything when `webpackPreload` is set to `true` but '@loadable/server'
// gathers these chunks and we render the appropriate tags in the HTML in
// `ServerHtml`.

const About = loadable(() =>
import(
/* webpackPreload: true, webpackChunkName: "About" */ '../../pages/StaticPages/About'
),
);

const ReviewGuide = loadable(() =>
import(
/* webpackPreload: true, webpackChunkName: "ReviewGuide" */ '../../pages/StaticPages/ReviewGuide'
),
);

type Props = {|
_config?: ConfigType,
|};
Expand Down
92 changes: 1 addition & 91 deletions src/amo/components/ServerHtml/index.js
Expand Up @@ -16,7 +16,6 @@ export default class ServerHtml extends Component {
static propTypes = {
appState: PropTypes.object.isRequired,
assets: PropTypes.object.isRequired,
chunkExtractor: PropTypes.object.isRequired,
component: PropTypes.element.isRequired,
htmlDir: PropTypes.string,
htmlLang: PropTypes.string,
Expand Down Expand Up @@ -111,93 +110,8 @@ export default class ServerHtml extends Component {
)}`;
}

renderStyles() {
const { chunkExtractor } = this.props;

return chunkExtractor
.getMainAssets('style')
.filter(
// We render the main bundle with `getScript()`, so we skip it here.
(asset) => asset.chunk !== APP_NAME,
)
.map((asset) => {
const sriProps = this.getSriProps(asset.filename);

return (
<link
data-chunk={asset.chunk}
href={asset.url}
key={asset.url}
rel="stylesheet"
type="text/css"
{...sriProps}
/>
);
});
}

renderPreLinks() {
const { chunkExtractor } = this.props;

return (
chunkExtractor
.getPreAssets()
// We want to retrieve the bundles with "webpackPreload: true" only, and
// not the main bundle (amo).
.filter((asset) => asset.type === 'childAsset')
// We return both "preload" and "prefetch" links to maximize browser
// support, even though both links don't have the same goal.
.map((asset) => [
<link
as={asset.scriptType}
data-parent-chunk={asset.chunk}
href={asset.url}
key={`preload-${asset.url}`}
rel="preload"
/>,
<link
as={asset.scriptType}
data-parent-chunk={asset.chunk}
href={asset.url}
key={`prefetch-${asset.url}`}
rel="prefetch"
/>,
])
);
}

renderAsyncScripts() {
const { chunkExtractor } = this.props;

return chunkExtractor
.getMainAssets('script')
.filter(
// We render the main bundle with `getScript()`, so we skip it here.
(asset) => asset.chunk !== APP_NAME,
)
.map((asset) => {
const sriProps = this.getSriProps(asset.filename);

return (
<script
async
data-chunk={asset.chunk}
key={asset.url}
src={asset.url}
{...sriProps}
/>
);
});
}

render() {
const {
appState,
chunkExtractor,
component,
htmlDir,
htmlLang,
} = this.props;
const { appState, component, htmlDir, htmlLang } = this.props;

// This must happen before Helmet.rewind() see
// https://github.com/nfl/react-helmet#server-usage for more info.
Expand All @@ -215,10 +129,8 @@ export default class ServerHtml extends Component {

<link rel="shortcut icon" href={this.getFaviconLink()} />
{head.link.toComponent()}
{this.renderPreLinks()}

{this.getStyle()}
{this.renderStyles()}

{head.script.toComponent()}
</head>
Expand All @@ -232,11 +144,9 @@ export default class ServerHtml extends Component {
type="application/json"
id="redux-store-state"
/>
{chunkExtractor.getRequiredChunksScriptElements()}

{this.getAnalytics()}
{this.getScript()}
{this.renderAsyncScripts()}
</body>
</html>
);
Expand Down
31 changes: 10 additions & 21 deletions src/amo/server/base.js
Expand Up @@ -16,12 +16,11 @@ import NestedStatus from 'react-nested-status';
import { END } from 'redux-saga';
import cookiesMiddleware from 'universal-cookie-express';
import WebpackIsomorphicTools from 'webpack-isomorphic-tools';
import { ChunkExtractor, ChunkExtractorManager } from '@loadable/server';

import log from 'amo/logger';
import { REGION_CODE_HEADER, createApiError } from 'amo/api';
import Root from 'amo/components/Root';
import { AMO_REQUEST_ID_HEADER, WEBPACK_ENTRYPOINT } from 'amo/constants';
import { AMO_REQUEST_ID_HEADER } from 'amo/constants';
import ServerHtml from 'amo/components/ServerHtml';
import * as middleware from 'amo/middleware';
import requestId from 'amo/middleware/requestId';
Expand Down Expand Up @@ -65,12 +64,6 @@ export function getPageProps({ store, req, res, config }) {
)
: {};

// Code-splitting.
const chunkExtractor = new ChunkExtractor({
stats: JSON.parse(fs.readFileSync(config.get('loadableStatsFile'))),
entrypoints: [WEBPACK_ENTRYPOINT],
});

// Check the lang supplied by res.locals.lang for validity
// or fall-back to the default.
const lang = isValidLang(res.locals.lang)
Expand Down Expand Up @@ -100,7 +93,6 @@ export function getPageProps({ store, req, res, config }) {

return {
assets: webpackIsomorphicTools.assets(),
chunkExtractor,
htmlLang: lang,
htmlDir: dir,
includeSri: isDeployed,
Expand Down Expand Up @@ -361,16 +353,14 @@ function baseServer(

const props = {
component: (
<ChunkExtractorManager extractor={pageProps.chunkExtractor}>
<Root
cookies={req.universalCookies}
history={history}
i18n={i18n}
store={store}
>
<App />
</Root>
</ChunkExtractorManager>
<Root
cookies={req.universalCookies}
history={history}
i18n={i18n}
store={store}
>
<App />
</Root>
),
};

Expand Down Expand Up @@ -564,8 +554,7 @@ export function runServer({
});
})
.catch((err) => {
// eslint-disable-next-line amo/only-log-strings
log.error('%o', { err });
log.error(`${err}`);

if (exitProcess) {
process.exit(1);
Expand Down

0 comments on commit 5f08286

Please sign in to comment.