Add CLI defaults for input and output directory #7966
Conversation
hzoo
added
the
PR: Polish 💅
|
I saw on some projects that I think the rules should be: if directory x exists pick another one. Note that the build destination directories are usually not committed and thus not exisinting. |
|
That's a good point about I don't think the rule you're proposing will work either though. The first time I build Maybe we should just use the path from the |
|
|
||
| outDirOptions.some(function(outDir) { | ||
| if (fs.existsSync(outDir)) { | ||
| return outDir; |
There was a problem hiding this comment.
This return isn't going to do what you want because it is inside of the .some callback, not inside getOutDir.
| const packageJson = JSON.parse(fs.readFileSync("package.json")); | ||
|
|
||
| if (packageJson && packageJson.main) { | ||
| return path.dirname(packageJson.main); |
There was a problem hiding this comment.
It's valid for a package.json#main to actually be a folder, so this will have issues, I'd suggest that we do
const basedir = process.cwd();
const filepath = require.resolve(path.resolve(basedir, packageJson.main));
return path.dirname(filepath);
so that we resolve the specific file first.
There was a problem hiding this comment.
Also we may want to skip here if the dirname that is returned is the root of the package and not a subfolder.
There was a problem hiding this comment.
For this second comment are you saying if dirname is the same as the basepath then we should just default to something like lib or dist (instead of writing the output to the root of the package)?
| @@ -124,6 +125,7 @@ commander.option( | |||
| commander.option( | |||
| "-d, --out-dir [out]", | |||
| "Compile an input directory of modules into an output directory", | |||
| getOutDir(), | |||
There was a problem hiding this comment.
This is going to break the check at https://github.com/iansu/babel/blob/3ec44323aa3477de09dc5b836a0f7233fc6b2967/packages/babel-cli/src/babel/index.js#L9 because there will always be a dir. We should invert that so that if the user specifies --out-file, it uses the file approach.
This is going to break Babel's ability to write to stdout:
because there will always be a file. We'll want to allow--out-file - to say "output to stdout".
| @@ -152,6 +154,10 @@ export default function parseArgv(args: Array<string>) { | |||
|
|
|||
| const errors = []; | |||
|
|
|||
| if (commander.args.length === 0) { | |||
| commander.args.push("src"); | |||
There was a problem hiding this comment.
This is going to break
babel/packages/babel-cli/src/babel/file.js
Line 228 in 6b91d64
babel - ...
where the - in place of a filename means it will read from stdin.
| } | ||
| } | ||
|
|
||
| outDirOptions.some(function(outDir) { |
There was a problem hiding this comment.
I think skipping the lib and dist and only using package.json might be reasonable, given the concern about lib, but it definitely has risks either way.
|
@loganfsmyth Thanks for the review. I will try to address those comments as well as some of the previous discussion later today. The stdin/stdout thing is definitely an issue. That's why a bunch of tests are currently failing. Using |
|
Using // index.js
module.exports = require('esm')(module)('./lib')+1 for @loganfsmyth's suggestion of skipping |
|
@iansu Any update on this? |
|
Yes! I made the suggested changes but haven't had a chance to test them yet. It kind of fell off my radar. Thanks for the reminder. I will try to push an update soon. |
|
@iansu Let me know if I can lend a hand. Still looking for my first contribution to babel. |
When running
babelwith no arguments input directory will default tosrcand output directory will default to:mainfield inpackage.jsonlibif it existsdistif it existslibWork in progress. To do: