Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: deprecate setup in favor of before and after hooks #1108

Merged
merged 2 commits into from Sep 26, 2017
Merged

feat: deprecate setup in favor of before and after hooks #1108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
febf76f
feat: deprecate setup in favor of before and after hooks
mrtnbroder Sep 20, 2017
40c6ece
Merge branch 'master' into mb/feature/middleware-before-after
shellscape Sep 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/node-api-middleware/server.js
View file
Open in desktop
Expand Up @@ -9,7 +9,7 @@ const server = new WebpackDevServer(compiler, {
stats: {
colors: true
},
setup(app) {
before(app) {
app.use((req, res, next) => {
console.log(`Using middleware for ${req.url}`);
next();
Expand Down
13 changes: 12 additions & 1 deletion lib/Server.js
View file
Open in desktop
Expand Up @@ -321,11 +321,19 @@ function Server(compiler, options) {
}
},

before: () => {
if (typeof options.before === 'function') { options.before(app, this); }
},

middleware: () => {
// include our middleware to ensure it is able to handle '/index.html' request after redirect
app.use(this.middleware);
},

after: () => {
if (typeof options.after === 'function') { options.after(app, this); }
},

headers: () => {
app.all('*', this.setContentHeaders.bind(this));
},
Expand All @@ -335,11 +343,13 @@ function Server(compiler, options) {
},

setup: () => {
log('Using "setup" is deprecated and will be removed in the next major version. Please use the "before" and "after" hooks instead.');
log('If "setup" was working fine for you until now, simply replace it with "before"');
if (typeof options.setup === 'function') { options.setup(app, this); }
}
};

const defaultFeatures = ['setup', 'headers', 'middleware'];
const defaultFeatures = ['before', 'setup', 'headers', 'middleware'];
Copy link
Contributor

Choose a reason for hiding this comment

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

@shellscape @mrtnbroder doesn't setup being in this array mean that everyone will see the deprecation warning no matter what? Shouldn't it not be in there but one of the things below this say if(options.setup) defaultFeatures.push('setup') or something? (or maybe keep it in but if options.setup is absent then remove it).

It seems that the loop on line 367 will always run the deprecation warning.

I'm asking because when i upgraded to 2.9 i noticed the deprecation warning even though I don't use this hook at all (though it may just be that a dependency of mine needs upgrading. I'm not totally sure)

Copy link
Contributor

Choose a reason for hiding this comment

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

either that or putting the logs on lines 346-347 inside the if on line 348

Copy link
Contributor

Choose a reason for hiding this comment

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

I made #1120 for this

if (options.proxy) { defaultFeatures.push('proxy', 'middleware'); }
if (contentBase !== false) { defaultFeatures.push('contentBaseFiles'); }
if (options.watchContentBase) { defaultFeatures.push('watchContentBase'); }
Expand All @@ -351,6 +361,7 @@ function Server(compiler, options) {
if (contentBase !== false) { defaultFeatures.push('contentBaseIndex'); }
// compress is placed last and uses unshift so that it will be the first middleware used
if (options.compress) { defaultFeatures.unshift('compress'); }
if (options.after) { defaultFeatures.push('after'); }

(options.features || defaultFeatures).forEach((feature) => {
features[feature]();
Expand Down
8 changes: 8 additions & 0 deletions lib/optionsSchema.json
View file
Open in desktop
Expand Up @@ -274,6 +274,14 @@
"description": "Exposes the Express server to add custom middleware or routes.",
"instanceof": "Function"
},
"before": {
"description": "Exposes the Express server to add custom middleware or routes before webpack-dev-middleware will be added.",
"instanceof": "Function"
},
"after": {
"description": "Exposes the Express server to add custom middleware or routes after webpack-dev-middleware got added.",
"instanceof": "Function"
},
"stats": {
"description": "Decides what bundle information is displayed.",
"anyOf": [
Expand Down
2 changes: 1 addition & 1 deletion test/Validation.test.js
View file
Open in desktop
Expand Up @@ -51,7 +51,7 @@ describe('Validation', () => {
' object { hot?, hotOnly?, lazy?, bonjour?, host?, allowedHosts?, filename?, publicPath?, port?, socket?, ' +
'watchOptions?, headers?, clientLogLevel?, overlay?, progress?, key?, cert?, ca?, pfx?, pfxPassphrase?, requestCert?, ' +
'inline?, disableHostCheck?, public?, https?, contentBase?, watchContentBase?, open?, useLocalIp?, openPage?, features?, ' +
'compress?, proxy?, historyApiFallback?, staticOptions?, setup?, stats?, reporter?, ' +
'compress?, proxy?, historyApiFallback?, staticOptions?, setup?, before?, after?, stats?, reporter?, ' +
'noInfo?, quiet?, serverSideRender?, index?, log?, warn? }'
]
}];
Expand Down