Skip to content

HMR plugin insertion for hot option and inline entry insertion in Server #1738

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

Merged
merged 13 commits into from
Mar 27, 2019
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions lib/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,61 @@ class Server {
throw new Error("'filename' option must be set in lazy mode.");
}

if (options.inline !== false) {
const findHMRPlugin = (config) => {
if (!config.plugins) {
return undefined;
}

return config.plugins.find(
(plugin) => plugin.constructor === webpack.HotModuleReplacementPlugin
);
};

const compilers = [];
const compilersWithoutHMR = [];
let webpackConfig;
if (compiler.compilers) {
webpackConfig = [];
compiler.compilers.forEach((compiler) => {
webpackConfig.push(compiler.options);
compilers.push(compiler);
if (!findHMRPlugin(compiler.options)) {
compilersWithoutHMR.push(compiler);
}
});
} else {
webpackConfig = compiler.options;
compilers.push(compiler);
if (!findHMRPlugin(compiler.options)) {
compilersWithoutHMR.push(compiler);
}
}

// it's possible that we should clone the config before doing
// this, but it seems safe not to since it actually reflects
// the changes we are making to the compiler
// important: this relies on the fact that addEntries now
// prevents duplicate new entries.
Server.addDevServerEntrypoints(webpackConfig, options);
compilers.forEach((compiler) => {
const config = compiler.options;
compiler.hooks.entryOption.call(config.context, config.entry);
});

// do not apply the plugin unless it didn't exist before.
if (options.hot || options.hotOnly) {
compilersWithoutHMR.forEach((compiler) => {
// addDevServerEntrypoints above should have added the plugin
// to the compiler options
const plugin = findHMRPlugin(compiler.options);
if (plugin) {
plugin.apply(compiler);
}
});
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I think better create helper/util for this and move this code from Server

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@evilebottnawi moved to utils/updateCompiler.js


this.stats =
options.stats && Object.keys(options.stats).length
? options.stats
Expand Down
14 changes: 12 additions & 2 deletions lib/utils/addEntries.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ function addEntries(config, options, server) {
// we're stubbing the app in this method as it's static and doesn't require
// a server to be supplied. createDomain requires an app with the
// address() signature.

const app = server || {
address() {
return { port: options.port };
Expand Down Expand Up @@ -35,13 +36,22 @@ function addEntries(config, options, server) {
const clone = {};

Object.keys(entry).forEach((key) => {
clone[key] = entries.concat(entry[key]);
// entry[key] should be a string here
clone[key] = prependEntry(entry[key]);
});

return clone;
}

return entries.concat(entry);
// 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) => {
if (!entriesClone.includes(newEntry)) {
entriesClone.push(newEntry);
}
});
return entriesClone;
};

// eslint-disable-next-line no-shadow
Expand Down
47 changes: 14 additions & 33 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
"marked": "^0.6.1",
"nyc": "^13.3.0",
"prettier": "^1.16.3",
"puppeteer": "^1.12.2",
"puppeteer": "^1.13.0",
"rimraf": "^2.6.2",
"standard-version": "^5.0.0",
"style-loader": "^0.23.1",
Expand Down
7 changes: 2 additions & 5 deletions test/Client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
const express = require('express');
const httpProxy = require('http-proxy-middleware');
const request = require('supertest');
const addEntries = require('../lib/utils/addEntries');
const helper = require('./helper');
const config = require('./fixtures/client-config/webpack.config');
const runBrowser = require('./helpers/run-browser');
Expand Down Expand Up @@ -33,8 +32,7 @@ describe('Client code', () => {
poll: true,
},
};
addEntries(config, options);
helper.start(config, options, done);
helper.startAwaitingCompilation(config, options, done);
});

afterAll(helper.close);
Expand Down Expand Up @@ -65,8 +63,7 @@ describe('Client code', () => {
expect(requestObj.url()).toMatch(
/^http:\/\/localhost:9000\/sockjs-node/
);
browser.close();
done();
browser.close().then(done);
});
page.goto('http://localhost:9000/main');
});
Expand Down
15 changes: 15 additions & 0 deletions test/Entry.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,21 @@ describe('Entry', () => {
]);
});

it('can prevent duplicate entries from successive calls', () => {
const webpackOptions = Object.assign({}, config);
const devServerOptions = { hot: true };

addEntries(webpackOptions, devServerOptions);
addEntries(webpackOptions, devServerOptions);

expect(webpackOptions.entry.length).toEqual(3);

const result = webpackOptions.entry.filter((entry) =>
normalize(entry).includes('webpack/hot/dev-server')
);
expect(result.length).toEqual(1);
});

it('supports entry as Function', () => {
const webpackOptions = Object.assign({}, configEntryAsFunction);
const devServerOptions = {};
Expand Down
Loading