Support modified entry point #30
Conversation
scripts/override-start.js
Outdated
| const hotClientIndex = config.entry.indexOf( hotClient ); | ||
| config.entry.splice(hotClientIndex, 1, require.resolve( '../overrides/webpackHotDevClient' ) ); | ||
|
|
||
| if ( config.entry instanceof Array ) { |
There was a problem hiding this comment.
Just a hint that the modern way of doing this is Array.isArray( config.entry ). While irrelevant here, this is also faster with modern browsers.
scripts/override-start.js
Outdated
| config.entry.splice( hotClientIndex, 1, require.resolve( '../overrides/webpackHotDevClient' ) ); | ||
| } | ||
|
|
||
| if ( config.entry instanceof Object ) { |
There was a problem hiding this comment.
This should be an else if, right?
There was a problem hiding this comment.
Yeah I had it as that but had forgotten Arrays are objects too, thought instanceof gave a more scoped result
scripts/override-start.js
Outdated
| Object.keys( config.entry ).forEach( key => { | ||
| const entry = config.entry[key]; | ||
|
|
||
| if ( entry instanceof String && entry === hotClient ) { |
There was a problem hiding this comment.
Why the extra type check when using === anyway?
scripts/override-start.js
Outdated
| } | ||
|
|
||
| if ( config.entry instanceof Object ) { | ||
| Object.keys( config.entry ).forEach( key => { |
There was a problem hiding this comment.
This looks like it should be Object.entries.
There was a problem hiding this comment.
Sometimes forget about node environment vs web! Thought Object.entries was still experimental so I've not made a habit of using it
scripts/override-start.js
Outdated
| config.entry[key] = require.resolve( '../overrides/webpackHotDevClient' ); | ||
| } | ||
|
|
||
| if ( entry instanceof Array ) { |
There was a problem hiding this comment.
Again else if, and Array.isArray.
scripts/override-start.js
Outdated
|
|
||
| if ( config.entry instanceof Array ) { | ||
| const hotClientIndex = config.entry.indexOf( hotClient ); | ||
| config.entry.splice( hotClientIndex, 1, require.resolve( '../overrides/webpackHotDevClient' ) ); |
There was a problem hiding this comment.
Why not check for >= 0 here? Like below in the forEach...
There was a problem hiding this comment.
Copy paste from the original code, but makes sense while we're making things robust
|
@tfrommen ace review thanks :) Will make those updates asap |
|
(Note that like create-react-app, you should aim for Node v8 compatibility, so some of these might not be available.) |
|
@rmccue not sure what version we need/want to support, but according to https://node.green/, both |
scripts/override-start.js
Outdated
| config.entry.splice( hotClientIndex, 1, require.resolve( '../overrides/webpackHotDevClient' ) ); | ||
| } | ||
| } else if ( config.entry instanceof Object ) { | ||
| Object.entries( config.entry ).forEach( ( key, entry ) => { |
There was a problem hiding this comment.
This needs to be ( [ key, entry ] ) => {.
scripts/override-start.js
Outdated
| config.entry[key] = require.resolve( '../overrides/webpackHotDevClient' ); | ||
| } | ||
|
|
||
| if ( Array.isArray( entry ) ) { |
There was a problem hiding this comment.
I don't think it strictly needs to be an else if, it won't affect the behaviour there. I usually find this style a bit more readable
|
Ha, I'd just found that same site. Pretty handy reference |
Fixes #29