Drop babel-node from build process #320
Conversation
Current coverage is 88.30%
|
| ```sh | ||
| node -v | ||
|
|
||
| v6.2.1 |
There was a problem hiding this comment.
Added a note to contributing to use Node 6 since babel-node is no longer transpiling build and config code.
3d0e933 to
d2ae62d
Compare
| @@ -27,7 +27,7 @@ const webpackConfig = { | |||
|
|
|||
| const webpackHotPath = `${config.compiler_public_path}__webpack_hmr` | |||
|
|
|||
| export const webpackHotMiddlewareEntry = `webpack-hot-middleware/client?${_.map({ | |||
| const webpackHotMiddlewareEntry = `webpack-hot-middleware/client?${_.map({ | |||
There was a problem hiding this comment.
This export was never used. I searched the code base as well as tested all the build scripts. All is well.
d2ae62d to
0e5593f
Compare
| @@ -42,23 +42,19 @@ module.exports = (karmaConfig) => { | |||
| }, | |||
| webpack: { | |||
| devtool: 'cheap-module-source-map', | |||
| module: { | |||
| ...webpackConfig.module, | |||
| module: Object.assign({}, webpackConfig.module, { | |||
There was a problem hiding this comment.
Would it make sense to use a spread operator here? I believe they're in node6.
There was a problem hiding this comment.
Man, I could have sworn it was though I'm on 6.2.1 and Node swears otherwise. Could be I need a flag or use strict or some other something enabled.
There was a problem hiding this comment.
There was a problem hiding this comment.
I know, but riddle me this:
spread.js
const objA = {
foo: 'bar'
}
const objB = { ...objA }$ node -v
v6.2.1
$ node spread.js
/Users/levithomason/src/stardust/spread.js:5
const objB = { ...objA }
^^^
SyntaxError: Unexpected token ...
at Object.exports.runInThisContext (vm.js:53:16)
at Module._compile (module.js:513:28)
at Object.Module._extensions..js (module.js:550:10)
at Module.load (module.js:458:32)
at tryModuleLoad (module.js:417:12)
at Function.Module._load (module.js:409:3)
at Function.Module.runMain (module.js:575:10)
at startup (node.js:160:18)
at node.js:456:3There must be some flag or some other config required. I just don't want to risk it not running for anyone when Object.assign() is available.
There was a problem hiding this comment.
OK, it works with arrays. Also seems to be with rest params, but not with objects....
source: http://node.green/
There was a problem hiding this comment.
Updated all arrays to use it and it's good. Thanks for the push on it!
|
Just some syntax comments, other than that LGTM! 🍂 |
0e5593f to
cf45482
Compare
cf45482 to
001ba69
Compare
* docs(contributing): add node 6 dep note * refactor(build): use node 6 compat (no babel-node)
This PR dials back the build and config syntax to work with vanilla node 6. This means no more
babel-nodein order to run Karma.This is good in general because it reduces complexity and deps. The main reason it was done though was in attempt for Windows compatibility. See #316, where we were having issues running karma by referencing the npm bin path. On windows, this needs to be the Windows version which I believe is
karma.cmd.Having removed babel-node means we also remove the need to reference the path directly. Windows should be able to execute the test script now and NPM should do its cross platform magic and run the correct version.
We shall see.