-
-
Notifications
You must be signed in to change notification settings - Fork 217
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
Improve cwd
option
#803
Improve cwd
option
#803
Conversation
|
||
let cwdStat; | ||
try { | ||
cwdStat = statSync(cwd); |
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 function is used both by execa()
and execaSync()
.
In principle, we could use two separate methods, one async and one sync. This would require having two versions of makeError()
, one async and one sync.
However, this function happens only when both:
- the child process failed
- the
cwd
option was set with a different value thanprocess.cwd()
So I thought it might be ok to keep it simple for the time being.
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.
So I thought it might be ok to keep it simple for the time being.
👍
|
||
export const normalizeFileUrl = file => file instanceof URL ? fileURLToPath(file) : file; | ||
|
||
export const fixCwdError = (originalMessage, cwd) => { |
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.
We cannot perform this cwd
validation check before process spawning because it involves making a stat
I/O call. Right now, execa()
spawns processes right away, and introducing a sync I/O call before spawning for every execa()
call is not worth it, just to improve the cwd
validation error message.
That being said, since process.cwd()
is synchronous, when the cwd
option default value is used, it is validated before process spawning.
Conflict |
Fixed. 👍 |
Fixes #798.
Fixes #799.