fix: avoid temporal optimize deps dirs #12582
Conversation
|
|
|
Almost there, windows is having issues. Probably missing a |
|
Looks like CI is faster in Windows now 👀 |
| files.push( | ||
| fsp.writeFile( | ||
| path.resolve(depsCacheDir, 'package.json'), | ||
| JSON.stringify({ type: 'module' }), |
There was a problem hiding this comment.
Would there be duplicated calls of JSON.stringify?
There was a problem hiding this comment.
I feel like doing a writeFile instead of checking existence before writing would be a bit faster, though perf doesn't matter a lot I think since these optimized files are only served to the browser.
There was a problem hiding this comment.
@sun0day changed it to be directly a string so we can have the same formatting as other files in the deps cache. We could use JSON.stringify({ type: 'module' }, null, 2) + '\n' too but the direct string feels better here.
@bluwy I added the check because I think the normal flow would be that this file is already there.
There was a problem hiding this comment.
Ok, I took that back after the latest changes as it was making the code more complex and I don't think it would make a diff
|
I found a trivial bug in main branch. v4.3.0-beta.0: I guess this PR will fix this bug indirectly. (haven't taken a look yet) |
|
Yes, that will be fixed. Any time you were killing the process while processing (or before committing), the temp dir will end up stale. |
|
It looks like the change log could do with updating: 4.3.0-beta.0 (2023-03-23) |
|
@mattydono this PR isn't released yet. Only commits that have been in a release are populated in the changelog. We're also reverting back to temporal dirs so we may clean it up #12622 |
Fixes #9986
Description
Use
write: falsewhen optimizing deps, and directly write files to the final deps dir when committed.What is the purpose of this pull request?