feat: replaces unzipper library with jszip #1086
Conversation
src/client/metadataApiRetrieve.ts
Outdated
| const extractPath = path.join(this.options.output, path.parse(name).name); | ||
| await dir.extract({ path: extractPath }); | ||
| fs.mkdirSync(extractPath); |
There was a problem hiding this comment.
checking my understanding: we need these mkdir/writeFile to be done sequentially to make sure the mkdir happens before any files that would go inside it?
There was a problem hiding this comment.
Yes, and I have no reason to believe that doing them async provides any perf boost.
src/client/metadataApiRetrieve.ts
Outdated
| for (const filePath of Object.keys(zip.files)) { | ||
| const zipObj = zip.file(filePath); | ||
| if (!zipObj || zipObj?.dir) { | ||
| fs.mkdirSync(path.join(extractPath, filePath)); |
There was a problem hiding this comment.
is there a reason we need the sync versions since the method is async?
There was a problem hiding this comment.
meta: I'm imagining something that iterates zip.files and does all the mkdir in one Promise.all and then all the writeFile in another Promise.all would be be faster.
There was a problem hiding this comment.
I don't think it would be faster, and I think it would make the code harder to read/debug. We can test it out though if you want.
| const extractPath = path.join(this.options.output, path.parse(name).name); | ||
| await dir.extract({ path: extractPath }); | ||
| fs.mkdirSync(extractPath, { recursive: true }); | ||
| for (const filePath of Object.keys(zip.files)) { |
|
|
||
| private constructor(directory: unzipper.CentralDirectory) { | ||
| private constructor(zip: JSZip) { |
There was a problem hiding this comment.
this avoids the declaration on 127 and the assignment on 131.
| private constructor(zip: JSZip) { | |
| private constructor(private zip: JSZip) { |
QA Notesbuilt and linked into plugin-source ✅ : ➜ du -sh force-app/main/default/staticresources/sample_data_brokers.json
3.6M force-app/main/default/staticresources/sample_data_brokers.json
➜ sf project deploy start
Deploying v58.0 metadata to test-wobioos3usus@example.com using the v59.0 SOAP API.
Deploy ID: 0Af8D00000pTsQZSA0
Status: Failed | ████████████████████████████████████████ | 1/1 Components (Errors:1) | 0/0 Tests (Errors:0)
Component Failures [1]
=========================================================================
| Type Name Problem Line:Column
| ───── ─────────────────── ───────────────────────────────── ───────────
| Error sample_data_brokers static resource cannot exceed 5MB ✅ : upload 🪟 ✅ : performed all of the same tests on a windows machine to verify |
What does this PR do?
replaces unzipper library with jszip. This will reduce dependencies since we are already using JSZip in this library for zipping metadata for a deploy. This affects StaticResouces unzipping in certain scenarios, unzipping retrieved metadata in mdapi format, and the ZipTreeContainer internal implementation.
What issues does this PR fix or reference?
@W-13520613@
Testing:
It's probably easiest to link this branch of the library to a CLI plugin, either plugin-source or plugin-deploy-retrieve
--unzipflag