gluestick-plugin-no-fouc: Remove Flash of Unstyled Content in develop…

…ment (#1036) * check for style assets in development also * initial impl of no-fouc plugin * no-fouc plugin refactor * no-fouc plugin test * no-fouc plugin readme * removed insignificant test case * fix typo in no-fouc plugin's README * fix typo in no-fouc plugin's README
TrueCar · Jun 12, 2017 · 13086d0 · 13086d0
1 parent 8cb822d
commit 13086d0
Show file tree

Hide file tree

Showing 8 changed files with 227 additions and 25 deletions.
diff --git a/packages/gluestick-plugin-no-fouc/.babelrc b/packages/gluestick-plugin-no-fouc/.babelrc
@@ -0,0 +1,10 @@
+{
+  "presets": [
+    ["env", {
+      "targets": {
+        "node": 6.3
+      }
+    }],
+    "stage-0"
+  ]
+}
diff --git a/packages/gluestick-plugin-no-fouc/.npmignore b/packages/gluestick-plugin-no-fouc/.npmignore
@@ -0,0 +1,5 @@
+**/__mocks__/**
+**/__tests__/**
+src/**
+coverage/**
+.babelrc
diff --git a/packages/gluestick-plugin-no-fouc/README.md b/packages/gluestick-plugin-no-fouc/README.md
@@ -0,0 +1,34 @@
+# `gluestick-plugin-no-fouc`
+
+Removes Flash of Unstyled Content (FOUC) in development.
+
+## How it works
+All styles will be extracted to css file using `ExtractTextWebpackPlugin` then, the file will be linked by server/renderer and added to `<head>` element. It sits side-by-side with `style-loader`, so you can still use HMR.
+
+__In production this plugin does nothing.__
+
+## How to use
+* Install plugin
+```
+npm install --save gluestick-plugin-no-fouc
+```
+* Define (and configure) plugin in `src/gluestick.plugins.js`:
+```javascript
+export default [
+  'gluestick-plugin-no-fouc'
+]
+
+// or
+
+export default [
+  {
+    plugin: 'gluestick-plugin-no-fouc',
+    options: {
+      filename: '[name].fouc-reducer.css'
+    },
+  },
+]
+```
+
+## Configuration
+- `filename`: `string` (default: `[name]-[contenthash].init-no-fouc.css`) - name of the file to which all styles will be extracted, can contain webpack tokens eg: `[name]`, `[hash]`, `[contenthash]` and so on
diff --git a/packages/gluestick-plugin-no-fouc/package.json b/packages/gluestick-plugin-no-fouc/package.json
@@ -0,0 +1,33 @@
+{
+  "name": "gluestick-plugin-no-fouc",
+  "version": "1.11.0",
+  "description": "Remove FOUC by extracting css into file and linking them to renderer page in development",
+  "main": "build/config.js",
+  "homepage": "https://github.com/TrueCar/gluestick",
+  "bugs": "https://github.com/TrueCar/gluestick/issues",
+  "license": "MIT",
+  "scripts": {
+    "test": "jest",
+    "test-coverage": "jest --coverage"
+  },
+  "author": "Todd Williams <toddsurfs@icloud.com>",
+  "repository": {
+    "type": "git",
+    "url": "git@github.com:TrueCar/gluestick.git"
+  },
+  "jest": {
+    "notify": true,
+    "testRegex": ".*/__tests__/.*\\.test\\.js$"
+  },
+  "peerDependencies": {
+    "gluestick": "1.11.0",
+    "extract-text-webpack-plugin": "2.x"
+  },
+  "devDependencies": {
+    "babel-preset-es2015": "6.22.0",
+    "babel-preset-stage-0": "6.22.0",
+    "extract-text-webpack-plugin": "2.1.2",
+    "gluestick": "1.11.0",
+    "jest": "18.1.0"
+  }
+}
diff --git a/packages/gluestick-plugin-no-fouc/src/__tests__/config.test.js b/packages/gluestick-plugin-no-fouc/src/__tests__/config.test.js
@@ -0,0 +1,101 @@
+jest.mock('extract-text-webpack-plugin', () => {
+  let calledTimes = 0;
+  class ExtractTestWebpackPlugin {
+    constructor(opts) {
+      this.opts = opts;
+      calledTimes++;
+    }
+  }
+  ExtractTestWebpackPlugin.extract = v => v;
+  ExtractTestWebpackPlugin.calledTimes = () => calledTimes;
+  return ExtractTestWebpackPlugin;
+});
+
+const ExtractTextWebpackPlugin = require('extract-text-webpack-plugin');
+const noFoucPlugin = require('../config');
+
+const nodeEnv = 'test';
+
+describe('gluestick no-fouc plugin', () => {
+  describe('in production', () => {
+    it('should do nothing', () => {
+      process.env.NODE_ENV = 'production';
+      const webpackClientConfig = {
+        plugins: [],
+        module: {
+          rules: [{
+            test: /\.(css)$/,
+            use: [],
+          }],
+        },
+      };
+      const { postOverwrites } = noFoucPlugin();
+      expect(postOverwrites.clientWebpackConfig(webpackClientConfig)).toEqual(webpackClientConfig);
+      expect(ExtractTextWebpackPlugin.calledTimes()).toBe(0);
+      process.env.NODE_ENV = nodeEnv;
+    });
+  });
+
+  describe('in development', () => {
+    beforeEach(() => {
+      process.env.NODE_ENV = nodeEnv;
+    });
+
+    it('should modify scss/css rules and add a plugin', () => {
+      const webpackClientConfig = {
+        plugins: [],
+        module: {
+          rules: [{
+            test: /\.(scss)$/,
+            use: [
+              'style-loader',
+              'css-loader',
+              'sass-loader',
+            ],
+          }, {
+            test: /\.(css)$/,
+            use: [
+              'style-loader',
+              'css-loader',
+            ],
+          }],
+        },
+      };
+      const { postOverwrites } = noFoucPlugin();
+      const modifiedConfig = postOverwrites.clientWebpackConfig(webpackClientConfig);
+      expect(modifiedConfig.plugins.length).toBe(1);
+      expect(modifiedConfig.plugins[0].opts.filename).toBeDefined();
+      expect(modifiedConfig.plugins[0].opts.allChunks).toBeTruthy();
+      expect(modifiedConfig.module.rules[0].use).toEqual([
+        { loader: 'style-loader' },
+        {
+          fallback: 'style-loader',
+          remove: false,
+          use: [
+            'css-loader',
+            'sass-loader',
+          ],
+        },
+      ]);
+      expect(modifiedConfig.module.rules[1].use).toEqual([
+        { loader: 'style-loader' },
+        {
+          fallback: 'style-loader',
+          remove: false,
+          use: [
+            'css-loader',
+          ],
+        },
+      ]);
+    });
+
+    it('shuld use provided filename from options', () => {
+      const filename = 'my-filename.css';
+      const { postOverwrites } = noFoucPlugin({ filename });
+      expect(postOverwrites.clientWebpackConfig({
+        plugins: [],
+        module: { rules: [] },
+      }).plugins[0].opts.filename).toEqual(filename);
+    });
+  });
+});
diff --git a/packages/gluestick-plugin-no-fouc/src/config.js b/packages/gluestick-plugin-no-fouc/src/config.js
@@ -0,0 +1,35 @@
+const ExtractTextPlugin = require('extract-text-webpack-plugin');
+
+const modifyLoader = ({ rules }, test) => {
+  const styleLoader = { loader: 'style-loader' };
+  const index = rules.findIndex(rule => rule.test.source === test.source);
+  if (index >= 0) {
+    const loaders = rules[index].use;
+    // eslint-disable-next-line no-param-reassign
+    rules[index].use = [styleLoader].concat(ExtractTextPlugin.extract({
+      fallback: loaders[0],
+      use: loaders.slice(1),
+      remove: false,
+    }));
+  }
+};
+
+const clientWebpackConfig = (filename = '[name]-[contenthash].init-no-fouc.css') => config => {
+  if (process.env.NODE_ENV !== 'production') {
+    modifyLoader(config.module, /\.(scss)$/);
+    modifyLoader(config.module, /\.(css)$/);
+    config.plugins.push(
+      new ExtractTextPlugin({
+        filename,
+        allChunks: true,
+      }),
+    );
+  }
+  return config;
+};
+
+module.exports = (options = {}) => ({
+  postOverwrites: {
+    clientWebpackConfig: clientWebpackConfig(options.filename),
+  },
+});
diff --git a/packages/gluestick/src/renderer/helpers/__tests__/linkAssets.test.js b/packages/gluestick/src/renderer/helpers/__tests__/linkAssets.test.js
@@ -12,21 +12,7 @@ const assets = {
 };
 
 describe('renderer/helpers/linkAssets', () => {
-  it('should return only script tag in development', () => {
-    const {
-      styleTags,
-      scriptTags,
-    } = linkAssets({}, 'main', assets, {});
-    expect(styleTags.length).toBe(0);
-    expect(scriptTags.length).toBe(1);
-    expect(scriptTags[0].type).toEqual('script');
-    expect(scriptTags[0].props.dangerouslySetInnerHTML.__html.includes('main-js')).toBeTruthy();
-    expect(scriptTags[0].props.dangerouslySetInnerHTML.__html.includes('vendor-js')).toBeTruthy();
-  });
-
-  it('should return scripts and style tags in production', () => {
-    const originalENV = process.env.NODE_ENV;
-    process.env.NODE_ENV = 'production';
+  it('should return scripts and style tags', () => {
     const {
       styleTags,
       scriptTags,
@@ -37,7 +23,6 @@ describe('renderer/helpers/linkAssets', () => {
     expect(scriptTags[0].props.dangerouslySetInnerHTML.__html.includes('main-js')).toBeTruthy();
     expect(scriptTags[0].props.dangerouslySetInnerHTML.__html.includes('vendor-js')).toBeTruthy();
     expect(styleTags[0].type).toEqual('link');
-    process.env.NODE_ENV = originalENV;
   });
 
   it('should resolve / entry name', () => {

diff --git a/packages/gluestick/src/renderer/helpers/linkAssets.js b/packages/gluestick/src/renderer/helpers/linkAssets.js
@@ -27,18 +27,17 @@ module.exports = (
   let key: number = 0;
   const entryPointName: string = filterEntryName(entryPoint);
 
-  if (process.env.NODE_ENV === 'production') {
-    const stylesHref: string = getAssetPathForFile(entryPointName, 'styles', assets);
+  const stylesHref: ?string = getAssetPathForFile(entryPointName, 'styles', assets);
+  if (stylesHref) {
     styleTags.push(
       <link key={key++} rel="stylesheet" type="text/css" href={stylesHref} />,
     );
-    // Need to explicitly check if there is CSS vendor bundle and include it
-    const vendorStylesHref: string = getAssetPathForFile('vendor', 'styles', assets);
-    if (vendorStylesHref) {
-      styleTags.push(
-        <link key={key++} rel="stylesheet" type="text/css" href={vendorStylesHref} />,
-      );
-    }
+  }
+  const vendorStylesHref: ?string = getAssetPathForFile('vendor', 'styles', assets);
+  if (vendorStylesHref) {
+    styleTags.push(
+      <link key={key++} rel="stylesheet" type="text/css" href={vendorStylesHref} />,
+    );
   }
 
   const vendorBundleHref: string = getAssetPathForFile('vendor', 'javascript', assets);