Sh/archiver to jszip swap #975
Conversation
| @@ -28,12 +28,12 @@ | |||
| "@salesforce/core": "^3.36.2", | |||
| "@salesforce/kit": "^1.9.2", | |||
| "@salesforce/ts-types": "^1.7.2", | |||
| "archiver": "^5.3.1", | |||
There was a problem hiding this comment.
be sure to take the types out of devDeps
src/client/metadataApiDeploy.ts
Outdated
| const zip = JSZip(); | ||
|
|
||
| const zipDirRecursive = (dir: string): void => { | ||
| const list = fs.readdirSync(dir); |
There was a problem hiding this comment.
readdir has an option for withFileTypes that will give you the isDirectory value without having to do another fs operation inside the loop.
|
|
||
| public constructor(rootDestination?: SourcePath) { | ||
| super(rootDestination); | ||
| void pipeline(this.zip, this.getOutputStream()); | ||
| const destination = rootDestination ? `for: ${rootDestination}` : 'in memory'; | ||
| this.logger.debug(`generating zip ${destination}`); |
There was a problem hiding this comment.
if the logger is only used in ZipWriter, why not create it on the lowest level so all the other writers don't suffer the perf impact of unused loggers.
then, if someone else needs it, they could hoist it higher in the hierarchy
There was a problem hiding this comment.
It's also used in StandardWriter
| ? this.addToZip(streamAsBuffer, writeInfo.output) | ||
| : // these will be zero-length files, which archiver supports via stream but not buffer | ||
| this.addToZip(writeInfo.source, writeInfo.output); | ||
| return this.addToZip(streamAsBuffer, writeInfo.output); |
There was a problem hiding this comment.
commits seem fine, no obvious perf impacts (looking especially at the sourceToZip across both OS).
https://forcedotcom.github.io/source-deploy-retrieve/perf-Linux/
https://forcedotcom.github.io/source-deploy-retrieve/perf-Windows/
* feat!: drop node 14/15 support * chore: use base tsconfig
…otcom/source-deploy-retrieve into sh/archiver-to-jszip-swap
|
QA notes: ✅ deploy a zip containing one empty text file 📓 Scenario: static resource has 1 zip file in it. That file is forceIgnored. we got an issue about this scenario once, and I saw the code change related to it. current SDR/PDR throws this error before sending anything to the API. 👎🏻 this PR sends the bad zip file to the API, get a deployment failure, and shows this error. I slightly prefer the old way (fail faster, especially if it's a large deployment) Still net improvement over archiver and happy to ship it. |
What does this PR do?
Replaces the archiver library with jszip.
What issues does this PR fix or reference?
@W-13196750@
QA Notes:
Be sure to test using both NodeJS v18.16.0 and v18.15.0, but primarily 18.16.0.