fix: only add client entry to web targets (#1775)

Author: mlrawlings

lib/options.json

@@ -172,6 +172,26 @@
     "inline": {
       "type": "boolean"
     },
+    "injectClient": {
+      "anyOf": [
+        {
+          "type": "boolean"
+        },
+        {
+          "instanceof": "Function"
+        }
+      ]
+    },
+    "injectHot": {
+      "anyOf": [
+        {
+          "type": "boolean"
+        },
+        {
+          "instanceof": "Function"
+        }
+      ]
+    },
     "disableHostCheck": {
       "type": "boolean"
     },

lib/utils/addEntries.js

@@ -18,45 +18,73 @@ function addEntries(config, options, server) {
     const domain = createDomain(options, app);
     const sockPath = options.sockPath ? `&sockPath=${options.sockPath}` : '';
     const sockPort = options.sockPort ? `&sockPort=${options.sockPort}` : '';
-    const entries = [
-      `${require.resolve('../../client/')}?${domain}${sockPath}${sockPort}`,
-    ];
+    const clientEntry = `${require.resolve(
+      '../../client/'
+    )}?${domain}${sockPath}${sockPort}`;
+    let hotEntry;
+
     if (options.hotOnly) {
-      entries.push(require.resolve('webpack/hot/only-dev-server'));
+      hotEntry = require.resolve('webpack/hot/only-dev-server');
     } else if (options.hot) {
-      entries.push(require.resolve('webpack/hot/dev-server'));
+      hotEntry = require.resolve('webpack/hot/dev-server');
     }
 
-    const prependEntry = (entry) => {
-      if (typeof entry === 'function') {
-        return () => Promise.resolve(entry()).then(prependEntry);
+    const prependEntry = (originalEntry, additionalEntries) => {
+      if (typeof originalEntry === 'function') {
+        return () =>
+          Promise.resolve(originalEntry()).then((entry) =>
+            prependEntry(entry, additionalEntries)
+          );
       }
 
-      if (typeof entry === 'object' && !Array.isArray(entry)) {
+      if (typeof originalEntry === 'object' && !Array.isArray(originalEntry)) {
         const clone = {};
 
-        Object.keys(entry).forEach((key) => {
+        Object.keys(originalEntry).forEach((key) => {
           // entry[key] should be a string here
-          clone[key] = prependEntry(entry[key]);
+          clone[key] = prependEntry(originalEntry[key], additionalEntries);
         });
 
         return clone;
       }
 
       // in this case, entry is a string or an array.
       // make sure that we do not add duplicates.
-      const entriesClone = entries.slice(0);
-      [].concat(entry).forEach((newEntry) => {
+      const entriesClone = additionalEntries.slice(0);
+      [].concat(originalEntry).forEach((newEntry) => {
         if (!entriesClone.includes(newEntry)) {
           entriesClone.push(newEntry);
         }
       });
       return entriesClone;
     };
 
+    // eslint-disable-next-line no-shadow
+    const checkInject = (option, config, defaultValue) => {
+      if (typeof option === 'boolean') return option;
+      if (typeof option === 'function') return option(config);
+      return defaultValue;
+    };
+
     // eslint-disable-next-line no-shadow
     [].concat(config).forEach((config) => {
-      config.entry = prependEntry(config.entry || './src');
+      const webTarget =
+        config.target === 'web' ||
+        config.target === 'webworker' ||
+        config.target == null;
+      const additionalEntries = checkInject(
+        options.injectClient,
+        config,
+        webTarget
+      )
+        ? [clientEntry]
+        : [];
+
+      if (hotEntry && checkInject(options.injectHot, config, true)) {
+        additionalEntries.push(hotEntry);
+      }
+
+      config.entry = prependEntry(config.entry || './src', additionalEntries);
 
       if (options.hot || options.hotOnly) {
         config.plugins = config.plugins || [];

test/Entry.test.js

@@ -277,4 +277,129 @@ describe('Entry', () => {
 
     expect(typeof webpackOptions.entry === 'function').toBe(true);
   });
+
+  it('only prepends devServer entry points to web targets by default', () => {
+    const webpackOptions = [
+      Object.assign({}, config),
+      Object.assign({ target: 'web' }, config),
+      Object.assign({ target: 'webworker' }, config),
+      Object.assign({ target: 'node' }, config) /* index:3 */,
+    ];
+
+    const devServerOptions = {};
+
+    addEntries(webpackOptions, devServerOptions);
+
+    // eslint-disable-next-line no-shadow
+    webpackOptions.forEach((webpackOptions, index) => {
+      const expectInline = index !== 3; /* all but the node target */
+
+      expect(webpackOptions.entry.length).toEqual(expectInline ? 2 : 1);
+
+      if (expectInline) {
+        expect(
+          normalize(webpackOptions.entry[0]).indexOf('client/index.js?') !== -1
+        ).toBeTruthy();
+      }
+
+      expect(normalize(webpackOptions.entry[expectInline ? 1 : 0])).toEqual(
+        './foo.js'
+      );
+    });
+  });
+
+  it('allows selecting compilations to inline the client into', () => {
+    const webpackOptions = [
+      Object.assign({}, config),
+      Object.assign({ target: 'web' }, config),
+      Object.assign({ name: 'only-include' }, config) /* index:2 */,
+      Object.assign({ target: 'node' }, config),
+    ];
+
+    const devServerOptions = {
+      injectClient: (compilerConfig) => compilerConfig.name === 'only-include',
+    };
+
+    addEntries(webpackOptions, devServerOptions);
+
+    // eslint-disable-next-line no-shadow
+    webpackOptions.forEach((webpackOptions, index) => {
+      const expectInline = index === 2; /* only the "only-include" compiler */
+
+      expect(webpackOptions.entry.length).toEqual(expectInline ? 2 : 1);
+
+      if (expectInline) {
+        expect(
+          normalize(webpackOptions.entry[0]).indexOf('client/index.js?') !== -1
+        ).toBeTruthy();
+      }
+
+      expect(normalize(webpackOptions.entry[expectInline ? 1 : 0])).toEqual(
+        './foo.js'
+      );
+    });
+  });
+
+  it('when hot, prepends the hot runtime to all targets by default', () => {
+    const webpackOptions = [
+      Object.assign({ target: 'web' }, config),
+      Object.assign({ target: 'node' }, config),
+    ];
+
+    const devServerOptions = {
+      // disable inlining the client so entry indexes match up
+      // and we can use the same assertions for both configs
+      injectClient: false,
+      hot: true,
+    };
+
+    addEntries(webpackOptions, devServerOptions);
+
+    // eslint-disable-next-line no-shadow
+    webpackOptions.forEach((webpackOptions) => {
+      expect(webpackOptions.entry.length).toEqual(2);
+
+      expect(
+        normalize(webpackOptions.entry[0]).includes('webpack/hot/dev-server')
+      ).toBeTruthy();
+
+      expect(normalize(webpackOptions.entry[1])).toEqual('./foo.js');
+    });
+  });
+
+  it('allows selecting which compilations to inject the hot runtime into', () => {
+    const webpackOptions = [
+      Object.assign({ target: 'web' }, config),
+      Object.assign({ target: 'node' }, config),
+    ];
+
+    const devServerOptions = {
+      injectHot: (compilerConfig) => compilerConfig.target === 'node',
+      hot: true,
+    };
+
+    addEntries(webpackOptions, devServerOptions);
+
+    // node target should have the client runtime but not the hot runtime
+    const webWebpackOptions = webpackOptions[0];
+
+    expect(webWebpackOptions.entry.length).toEqual(2);
+
+    expect(
+      normalize(webWebpackOptions.entry[0]).indexOf('client/index.js?') !== -1
+    ).toBeTruthy();
+
+    expect(normalize(webWebpackOptions.entry[1])).toEqual('./foo.js');
+
+    // node target should have the hot runtime but not the client runtime
+    const nodeWebpackOptions = webpackOptions[1];
+
+    expect(nodeWebpackOptions.entry.length).toEqual(2);
+
+    expect(
+      normalize(nodeWebpackOptions.entry[0]).includes('webpack/hot/dev-server')
+    ).toBeTruthy();
+
+    expect(normalize(nodeWebpackOptions.entry[1])).toEqual('./foo.js');
+  });
 });

test/UniversalCompiler.test.js

@@ -0,0 +1,48 @@
+'use strict';
+
+const request = require('supertest');
+const helper = require('./helper');
+const config = require('./fixtures/universal-compiler-config/webpack.config');
+
+describe('UniversalCompiler', () => {
+  let server;
+  let req;
+  beforeAll((done) => {
+    server = helper.start(config, { inline: true }, done);
+    req = request(server.app);
+  });
+
+  afterAll(helper.close);
+
+  it('client bundle should have the inlined the client runtime', (done) => {
+    req
+      .get('/client.js')
+      .expect('Content-Type', 'application/javascript; charset=UTF-8')
+      .expect(200)
+      .end((err, res) => {
+        if (err) {
+          return done(err);
+        }
+        expect(res.text).toContain('Hello from the client');
+        expect(res.text).toContain('webpack-dev-server/client');
+        done();
+      });
+  });
+
+  it('server bundle should NOT have the inlined the client runtime', (done) => {
+    // we wouldn't normally request a server bundle
+    // but we'll do it here to check the contents
+    req
+      .get('/server.js')
+      .expect('Content-Type', 'application/javascript; charset=UTF-8')
+      .expect(200)
+      .end((err, res) => {
+        if (err) {
+          return done(err);
+        }
+        expect(res.text).toContain('Hello from the server');
+        expect(res.text).not.toContain('webpack-dev-server/client');
+        done();
+      });
+  });
+});

test/fixtures/universal-compiler-config/client.js

@@ -0,0 +1,3 @@
+'use strict';
+
+console.log('Hello from the client');

test/fixtures/universal-compiler-config/server.js

@@ -0,0 +1,3 @@
+'use strict';
+
+console.log('Hello from the server');

test/fixtures/universal-compiler-config/webpack.config.js

@@ -0,0 +1,25 @@
+'use strict';
+
+module.exports = [
+  {
+    mode: 'development',
+    context: __dirname,
+    entry: './client.js',
+    output: {
+      path: '/',
+      filename: 'client.js',
+    },
+    node: false,
+  },
+  {
+    mode: 'development',
+    context: __dirname,
+    target: 'node',
+    entry: './server.js',
+    output: {
+      path: '/',
+      filename: 'server.js',
+    },
+    node: false,
+  },
+];