-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
WIP: Add a 'run' script #245
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
/** | ||
* Copyright (c) 2015-present, Facebook, Inc. | ||
* All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
*/ | ||
|
||
var fs = require('fs'); | ||
var os = require('os'); | ||
var path = require('path'); | ||
var webpack = require('webpack'); | ||
|
||
var paths = require('../config/paths'); | ||
|
||
// Hacky command-line parsing. All flags are just ignored and there | ||
// may be no args. | ||
// TODO: support passing args to the script we're running | ||
var nonflags = process.argv.filter(function(x) { return x[0] !== '-'; }); | ||
if (nonflags.length != 3) { | ||
console.error('Usage: react-scripts run foo.js'); | ||
console.error( | ||
'foo.js is your script, resolved relative to the src directory.') | ||
process.exit(1); | ||
} | ||
var scriptName = nonflags[2]; | ||
var pathToEntry = path.join(paths.appSrc, scriptName); | ||
|
||
// Figure out what all the node_modules are. | ||
// We don't want to compile the node_modules, but we still want to | ||
// do a commonjs require for them. | ||
var nodeModules = {}; | ||
fs.readdirSync(paths.appNodeModules).filter(function(x) { | ||
return ['.bin'].indexOf(x) === -1; | ||
}).forEach(function(mod) { | ||
nodeModules[mod] = 'commonjs ' + mod; | ||
}); | ||
|
||
// Construct our script in a temporary file. | ||
var outputPath = os.tmpDir(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: Windows doesn't automatically clean out its temporary directory, and on platforms where /tmp is mounted tmpfs (RAM-backed), this could use additional RAM unnecessarily. |
||
var outputFile = 'entry.js'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems kinda weird to call it 'entry.js' when it's technically the output 😉 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I run two scripts at once, would they both be written as eg There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I initially thought it would be in a new temporary directory each time but it looks like I am mistaken on that. Yeah this leads to a race condition & I should fix it. |
||
|
||
// Configure webpack. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was there a specific reason why you chose to write the webpack config inline rather than breaking it out into a separate file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could do: // webpack.config.js
module.exports = (scriptName) => {
return {
// adapt based on scriptName here
};
}; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True. Is that aesthetically superior to just leaving it all in one file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It has the benefit of matching the new syntax allowed in Webpack 2, where configs can actually be functions that return the config object. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've always grouped them because it makes it easy to sync the different webpack configs. (i.e. have a |
||
// TODO: support sourcemaps | ||
// TODO: figure out if we want to automatically polyfill fetch | ||
var config = { | ||
entry: [ | ||
pathToEntry, | ||
], | ||
target: 'node', | ||
output: { | ||
libraryTarget: 'commonjs2', | ||
path: outputPath, | ||
filename: outputFile | ||
}, | ||
externals: nodeModules, | ||
resolve: { | ||
extensions: ['', '.js', '.json'], | ||
}, | ||
resolveLoader: { | ||
root: paths.ownNodeModules, | ||
moduleTemplates: ['*-loader'] | ||
}, | ||
module: { | ||
loaders: [ | ||
{ | ||
test: /\.js$/, | ||
include: paths.appSrc, | ||
loader: 'babel', | ||
query: require('../config/babel.dev') | ||
}, | ||
{ | ||
test: /\.css$/, | ||
include: [paths.appSrc, paths.appNodeModules], | ||
loader: 'null' | ||
}, | ||
{ | ||
test: /\.json$/, | ||
include: [paths.appSrc, paths.appNodeModules], | ||
loader: 'json' | ||
}, | ||
{ | ||
test: /\.(jpg|png|gif|eot|svg|ttf|woff|woff2)$/, | ||
include: [paths.appSrc, paths.appNodeModules], | ||
loader: 'null', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. People probably won't need them, but is there a specific reason to null out all these? What if they're writing a script that processes images? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think is the right behavior here? AFAICT there is no standard behavior for requiring an image into a node.js environment, so I just wanted the imports to neither crash nor do anything. |
||
}, | ||
{ | ||
test: /\.(mp4|webm)$/, | ||
include: [paths.appSrc, paths.appNodeModules], | ||
loader: 'null' | ||
} | ||
] | ||
}, | ||
}; | ||
|
||
// Compile the script | ||
webpack(config).run(function(err, stats) { | ||
if (err) { | ||
console.error('error:', err.code, err.message); | ||
console.error(err); | ||
process.exit(2); | ||
} | ||
if (stats.hasErrors()) { | ||
var errors = stats.toJson().errors; | ||
console.error('stats has', errors.length, 'errors:'); | ||
for (var i = 0; i < errors.length; i++) { | ||
console.error(errors[i]); | ||
} | ||
process.exit(3); | ||
} | ||
|
||
var scriptPath = path.resolve(outputPath, outputFile); | ||
require(scriptPath); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't cover eg
require("lodash/fp/extend")
(it'll get bundled instead of marked as an external).If you want, you could just use webpack-node-externals to handle this part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I hadn't tested requires with a
/
in them. Thanks for pointing me to webpack-node-externals - I'll take a look at that.