Skip to content

Commit

Permalink
fix: only add entries after compilers have been created (#1774)
Browse files Browse the repository at this point in the history
  • Loading branch information
@DylanPiercey @evilebottnawi
DylanPiercey authored and evilebottnawi committed Apr 9, 2019
1 parent 66b04a9 commit b31cbaa
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 3 deletions.
3 changes: 0 additions & 3 deletions bin/webpack-dev-server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const webpack = require('webpack');
const options = require('./options');
const Server = require('../lib/Server');

const addEntries = require('../lib/utils/addEntries');
const colors = require('../lib/utils/colors');
const createConfig = require('../lib/utils/createConfig');
const createDomain = require('../lib/utils/createDomain');
Expand Down Expand Up @@ -140,8 +139,6 @@ function processOptions(config) {
function startDevServer(config, options) {
const log = createLogger(options);

addEntries(config, options);

let compiler;

try {
Expand Down
59 changes: 59 additions & 0 deletions test/Server.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,71 @@
'use strict';

const { relative, sep } = require('path');
const webpack = require('webpack');
const request = require('supertest');
const Server = require('../lib/Server');
const config = require('./fixtures/simple-config/webpack.config');
const helper = require('./helper');

describe('Server', () => {
describe('addEntries', () => {
it('add hot option', () => {
return new Promise((res) => {
// eslint-disable-next-line
const Server = require('../lib/Server');
const compiler = webpack(config);
const server = new Server(compiler, {
hot: true,
});

expect(
server.middleware.context.compiler.options.entry.map((p) => {
return relative('.', p).split(sep);
})
).toMatchSnapshot();
expect(
server.middleware.context.compiler.options.plugins
).toMatchSnapshot();

compiler.hooks.done.tap('webpack-dev-server', () => {
server.close(() => {
res();
});
});

compiler.run(() => {});
});
});

it('add hotOnly option', () => {
return new Promise((res) => {
// eslint-disable-next-line
const Server = require('../lib/Server');
const compiler = webpack(config);
const server = new Server(compiler, {
hotOnly: true,
});

expect(
server.middleware.context.compiler.options.entry.map((p) => {
return relative('.', p).split(sep);
})
).toMatchSnapshot();
expect(
server.middleware.context.compiler.options.plugins
).toMatchSnapshot();

compiler.hooks.done.tap('webpack-dev-server', () => {
server.close(() => {
res();
});
});

compiler.run(() => {});
});
});
});

// issue: https://github.com/webpack/webpack-dev-server/issues/1724
describe('express.static.mine.types', () => {
beforeEach(() => {
Expand Down
60 changes: 60 additions & 0 deletions test/__snapshots__/Server.test.js.snap
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,65 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Server addEntries add hot option 1`] = `
Array [
Array [
"client",
"index.js?http:",
"localhost",
],
Array [
"node_modules",
"webpack",
"hot",
"dev-server.js",
],
Array [
"foo.js",
],
]
`;

exports[`Server addEntries add hot option 2`] = `
Array [
HotModuleReplacementPlugin {
"fullBuildTimeout": 200,
"multiStep": undefined,
"options": Object {},
"requestTimeout": 10000,
},
]
`;

exports[`Server addEntries add hotOnly option 1`] = `
Array [
Array [
"client",
"index.js?http:",
"localhost",
],
Array [
"node_modules",
"webpack",
"hot",
"only-dev-server.js",
],
Array [
"foo.js",
],
]
`;

exports[`Server addEntries add hotOnly option 2`] = `
Array [
HotModuleReplacementPlugin {
"fullBuildTimeout": 200,
"multiStep": undefined,
"options": Object {},
"requestTimeout": 10000,
},
]
`;

exports[`Server stats should works with difference stats values (contains 'hash', 'assets', 'warnings' and 'errors') 1`] = `
Array [
"errors",
Expand Down

8 comments on commit b31cbaa

@kelihansen
Copy link

Choose a reason for hiding this comment

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

This actually seems to have been a breaking change for us, but I'm still trying to pinpoint why.

@alexander-akait
Copy link
Member

Choose a reason for hiding this comment

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

Nope we already do this before starts webpack-dev-server, so it is not breaking change, maybe you have invalid configuration previously

@kelihansen
Copy link

Choose a reason for hiding this comment

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

It's possible, and that's part of what I'm trying to determine, but the fact of the matter is that without those two lines, we get Cannot read property 'call' of undefined at __webpack_require__ in the browser console, but if I go into my node_modules and put the lines back in, the error disappears.

@alexander-akait
Copy link
Member

Choose a reason for hiding this comment

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

You have disabled hmr based on error, please provide minimum reproducible test repo, thanks

@rbuetzer
Copy link

Choose a reason for hiding this comment

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

@kelihansen Did you gain any insights in the meantime? I ran into the same issue. Also, it seems that version 3.3.1 features breaking changes for other people as well: webpack/webpack#6094 (comment)

@kelihansen
Copy link

Choose a reason for hiding this comment

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

Bizarrely, our issue disappeared when we changed the entry property of our webpack config from a string to an array. I never had time to understand why.

@alexander-akait
Copy link
Member

Choose a reason for hiding this comment

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

@rbuetzer can you open issue with reproducible test repo?

@rbuetzer
Copy link

Choose a reason for hiding this comment

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

Woah, using an array for the entry property did the trick. Thanks a lot, @kelihansen!

@evilebottnawi: I don't have the time to create a reproducible test repo from our private project, I have however created #2231, which includes my webpack config. Hope this is still helpful.

Please sign in to comment.