Conversation
ae3b785 to
c965ba9
Compare
|
Thanks for digging into this. Can we pull @ashaffer into the discussion? The original patch is from browserify/browserify#1543 |
That's interesting, I didn't know about that. However, I originally didn't have that call inside there and then moved it in because its possible for the parent process to change directories on you, thus making your cache invalid. Is there a way we could be notified of chdirs? I don't know if we can just ignore chdirs entirely, unfortunately. Browserify is async, and the working directory could be changed in the middle of a browserify. This would be kinda weird - but I can maybe see a plugin doing it for some odd reason. It's not obvious to me that this would be safe. Thoughts?
Good point. That is completely unnecessary, I have no memory of why that is in there. I'll fix that now. |
|
What if we exposed the ability to reset the cache to plugins? Then if a plugin hits this problem it can do its own check and clear? |
|
Could we pass the cwd as an argument so it can be done once at the start of the dependency process? |
|
@substack that seems like the right approach to me. It does seem possible it will break some people's existing (hacky) workflows, but that seems ultimately more correct. |
The
cached-path-relativemodule hitsprocess.cwd()a lot, I know this might be an outlier but under certain condition its possible to end up withError: ENFILE: file table overflow, uv_cwd.cached-path-relativealso callsbindonpath.relative(for no apparent reason) - which is known to be slow.Since
module-deps(norbrowserify) does aprocess.chdir(not has any need to) we can reduce the code down to a var declaration and a five line function.