Some build refactors/improvements #5440
Conversation
| @@ -49,7 +49,7 @@ function makeReleaseCopies( Release ) { | |||
| ].forEach( ( { filesMap, cdnFolder } ) => { | |||
| shell.mkdir( "-p", cdnFolder ); | |||
|
|
|||
| Object.keys( filesMap ).forEach( key => { | |||
| Object.keys( filesMap ).forEach( ( key ) => { | |||
There was a problem hiding this comment.
Ran prettier on this file
build/release/cdn.js
Outdated
| .readFileSync( builtFile, "utf8" ) | ||
| .replace( | ||
| /"file":"([^"]+)"/, | ||
| "\"file\":\"" + unpathedFile.replace( /\.min\.map/, ".min.js\"" ) |
There was a problem hiding this comment.
It'd probably be more readable as a template literal; quote escapes would also disappear:
| "\"file\":\"" + unpathedFile.replace( /\.min\.map/, ".min.js\"" ) | |
| `"file":"${ unpathedFile.replace( /\.min\.map/, ".min.js\"" ) }` |
I wonder, though - such replaces are risky, it's easy to match incorrectly and that can be hard to find. Why not parse this JSON, fix the fields and serialize again? We're doing something similar in SWC minify, although without the initial parsing as we already get an object.
There was a problem hiding this comment.
I'm fine making this a template literal, but I worry about changing the logic here as it would require testing releases. How about we change that with the release PR? I made a note of it.
There was a problem hiding this comment.
LGTM. The comment #5440 (comment) is yet to be addressed but that's going to happen in a separate release PR.
Summary
+slim.to+in the regex. It was only matching on the latter even for the slim file, which made it so comparisons between a dirty build and the saved branch were always off by 5-6 bytes.Checklist