Add overwrite protection to parent directory #358
Conversation
|
hmm, tests failing but not locally |
oligriffiths
force-pushed
the
oli/parent-output-path
branch
2 times, most recently
from
00a816e to
50db293
Compare
|
@rwjblue do you know of any reason to allow |
test/cli_test.js
Outdated
| @@ -36,6 +36,9 @@ describe('cli', function() { | |||
| }); | |||
|
|
|||
| describe('build', function() { | |||
| beforeEach(function() { | |||
| rimraf.sync('dist'); | |||
There was a problem hiding this comment.
We should generally not have to cleanup both before and after like this. Having to cleanup before generally indicates some other test is leaving us in an invalid starting state...
|
I think I found the problem. |
lib/cli.js
Outdated
| ); | ||
| process.exit(1); | ||
| } | ||
| } | ||
|
|
||
| function isParentDirectory(outputPath) { |
There was a problem hiding this comment.
maybe you can use findup-sync to detect project root by locating package.json ?
oligriffiths
force-pushed
the
oli/parent-output-path
branch
from
c4cfc14 to
6c181bf
Compare
oligriffiths
force-pushed
the
oli/parent-output-path
branch
from
b8d799e to
80efdec
Compare
| function guardOutputDir(outputDir, removeExisting) { | ||
| if (fs.existsSync(outputDir)) { | ||
| if (removeExisting) { | ||
| rimraf.sync(outputDir); |
There was a problem hiding this comment.
Removing rimraf as broccoli already handles doing a diff of the output directory and the new files to be added/removed, so rimraf actually makes it less efficient. https://github.com/broccolijs/broccoli/blob/master/lib/cli.js#L104
|
Good work, thank you!! |
This PR ensures that if using the
--overwriteoption andtargetis a parent directory of the project, the process will exit(1).This is inline with ember-cli.
Also adds some more tests for
--overwriteoption.Question, why do we even allow the
targetdirectory to be a literal parent of the project, I don't really see why you would want that?