fix: disable caching when babel could not read/write cache#10557
fix: disable caching when babel could not read/write cache#10557nicolo-ribaudo merged 7 commits intobabel:masterfrom
Conversation
|
Is it possible to add a test somehow? |
| ); | ||
| cacheDisabled = true; | ||
| break; | ||
| case "EROFS": |
There was a problem hiding this comment.
Disclaimer: I have not tested on a readonly filesystem, this error code is copied from man 2 open
[EROFS] The named file resides on a read-only file system, and the file is to be modified.
|
@nicolo-ribaudo Added test on permission issues. On the readonly file system error, I think it is hard to reproduce on unit testing. |
| load(); | ||
|
|
||
| process.nextTick(() => { | ||
| process.nextTick(() => { |
There was a problem hiding this comment.
[Q]: Why isn't one nextTick enough?
There was a problem hiding this comment.
[A]: I thought the execution order of next tick tasks is not defined. Apparently I am wrong after reading https://nodejs.org/docs/latest/api/process.html#process_process_nexttick_callback_args, thank you for reminding me!
|
@JLHwung The failing test on windows it a test of this PR. |
|
@nicolo-ribaudo The failing test on Windows is due to the following caveats
I guess that the read error could only occur when the file is owned by another user and have ACL settings excluding current user to read it. There is no As it is difficult to construct such example on Windows CI, I disable this test on Windows. |
In this PR we catch the IO error and disable caching when permission issue arises, or it the cache path resides in a readonly filesystem.
The following
existsSyncusageis also replaced as it introduces duplicate IO operations. In general
fs.existsSyncshould only be usedIn the future I will open a separate PR to replace all similar usage in our codebase.