Navigation Menu

Skip to content
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

Ensure path to outfile exists before opening stream #995

Merged

Conversation

jackwanders
Copy link
Contributor

I found today that the --outfile option for browserify will fail when the user attempts to output the browserify output to a file in a non-existent directory. For example, when running

$ browserify ./my/src/script.js --outfile=./dist/bundle.js

the following error occurs when ./dist/ doesn't exist:

events.js:72
        throw er; // Unhandled 'error' event
              ^
Error: ENOENT, open './dist/bundle.js'
Warning: Command failed:
events.js:72
        throw er; // Unhandled 'error' event
              ^
Error: ENOENT, open './dist/bundle.js'
 Use --force to continue.

This seems like a common enough case (e.g. browserify may run during a build that contains a clean step that blows away the intended destination directory) that I believe browserify should simply make the directory rather than blow up when fs.createWriteStream fails.

This pull request uses mkdirp to ensure the outfile directory exists before opening the stream, and modifies a test case that would fail without this change.

@twisterghost
Copy link

👍

This will be immensely helpful when using browserify as a step in a build process.

@rubennorte
Copy link

I'd also appreciate that this PR is merged. Now we have to manually create the output directories as a previous build step and is kind of annoying.

@mysticatea
Copy link

👍

@IanVS
Copy link

IanVS commented Dec 29, 2015

It's been more than a year, and this is still a problem. Is there a reason this hasn't been reviewed or merged?

@jcpst
Copy link

jcpst commented Feb 16, 2016

Would love this feature to be merged.

@jasonkarns
Copy link

Hoping this gets updated and merged. This is still a problem. (Also worth noting that babel and rollup both happily create any necessary output directories, which means any pre-emptive directory creation in npm-scripts is solely to appease browserify.)

@goto-bus-stop
Copy link
Member

Oh yeah I think this is great — @jackwanders would you like to update the PR to current master? Otherwise I can do it too

@jackwanders
Copy link
Contributor Author

@goto-bus-stop I'd be happy to update this PR. Given the holidays, I trust waiting a few days is okay, given the age of the PR.

@goto-bus-stop
Copy link
Member

goto-bus-stop commented Dec 28, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants