Switch to yauzl for unzipping template files

Currently, template files come into the user's site like this (see handleInstall() in packages/pushkin-cli/src/index.js):

    // Unzip the template files into the appropriate directory
    if (verbose) console.log(`Unzipping template files into ${templateType} directory`);
    try {
      // On some systems, we noticed the unzip command seemingly trying to execute before the install command was finished.
      // The attempted fix is the following:
      // Check if the path to the zip archive in node_modules exists
      if (fs.existsSync(`node_modules/${templateName}/build/template.zip`)) {
        // If so, unzip it
        execSync(`unzip node_modules/${templateName}/build/template.zip ${ templateType === 'experiment' ? `-d ${path.join(config.experimentsDir, shortName)}` : '' }`);
      } else {
        // Otherwise, wait 5 seconds and try to unzip it
        await new Promise(resolve => setTimeout(resolve, 5000));
        execSync(`unzip node_modules/${templateName}/build/template.zip ${ templateType === 'experiment' ? `-d ${path.join(config.experimentsDir, shortName)}` : '' }`);
      }
    } catch (e) {
      console.error(`Problem unzipping ${templateType} template files`);
      throw e;
    }

In addition to the setTimeout hack, I've occasionally gotten weird errors during unzipping like ENOBUFS. Staying within Node by using something like yauzl to unzip the template files would help us streamline and cut out excess calls to execSync.

Sounds good, but please provide detail on what exactly this would help with. Keeping in mind that it is adding more dependencies.

On April 3, 2024, Sebastian Waz @.***> wrote:

Currently, template files come into the user's site like this (see handleInstall() in packages/pushkin-cli/src/index.js):

// Unzip the template files into the appropriate directory if (verbose) console.log(Unzipping template files into ${templateType} directory); try { // On some systems, we noticed the unzip command seemingly trying to execute before the install command was finished. // The attempted fix is the following: // Check if the path to the zip archive in node_modules exists if (fs.existsSync(node_modules/${templateName}/build/template.zip)) { // If so, unzip it execSync(unzip node_modules/${templateName}/build/template.zip ${ templateType === 'experiment' ?-d ${path.join(config.experimentsDir, shortName)}: '' }); } else { // Otherwise, wait 5 seconds and try to unzip it await new Promise(resolve => setTimeout(resolve, 5000)); execSync(unzip node_modules/${templateName}/build/template.zip ${ templateType === 'experiment' ?-d ${path.join(config.experimentsDir, shortName)}: '' }); } } catch (e) { console.error(Problem unzipping ${templateType} template files); throw e; } In addition to the setTimeout hack, I've occasionally gotten weird errors during unzipping like ENOBUFS. Staying within Node by using something like yauzl https://www.npmjs.com/package/yauzl to unzip the template files would help us streamline and cut out excess calls to execSync.

— Reply to this email directly, view it on GitHub https://github.com/pushkin-consortium/pushkin/issues/322, or unsubscribe https://github.com/notifications/unsubscribe- auth/ABOVVY3AXOWC523PGUCF3GLY3Q5IBAVCNFSM6AAAAABFVYY7D6VHI2DSMVQWIX3LMV43ASLTON2WKOZSGIZDGNJSHA2TOOA. You are receiving this because you are subscribed to this thread.Message ID: @.***>

This is mostly about aligning with the best practice of staying within node instead of using execSync. The current code does work (I wouldn't call this issue high priority), but we had to add the setTimeout business because we noticed on @hunterschep's system, execSync wasn't blocking execution long enough, and the CLI was trying to read template files which hadn't yet made it into the site.

Also before 973bb92667497df54d41bea5f85061d855c89f06, sometimes installing a template from path could fail if you had previously been doing development that involved installing packages in one of the internal components (e.g. front-end in the site template). When the CLI was trying to unzip a template that already had node_modules in it, you could get an ENOBUFS error -- I'm guessing because node_modules had a lot subfolders. Worth noting that this could only happen when installing a template from path; because of the publication workflow, templates would never be published to npm already containing node_modules.

If future templates got larger -- maybe a template with a lot of static resources -- I could see this issue coming up again.

Great. Sounds worth doing.

On April 3, 2024, Sebastian Waz @.***> wrote:

This is mostly about aligning with the best practice of staying within node instead of using execSync. The current code does work (I wouldn't call this issue high priority), but we had to add the setTimeout business because we noticed on @hunterschep https://github.com/hunterschep's system, execSync wasn't blocking execution long enough, and the CLI was trying to read template files which hadn't yet made it into the site.

Also before 973bb92 https://github.com/pushkin- consortium/pushkin/commit/973bb92667497df54d41bea5f85061d855c89f06, sometimes installing a template from path could fail if you had previously been doing development that involved installing packages in one of the internal components (e.g. front-end in the site template). When the CLI was trying to unzip a template that already had node_modules in it, you could get an ENOBUFS error -- I'm guessing because node_modules had a lot subfolders. Worth noting that this could only happen when installing a template from path; because of the publication workflow, templates would never be published to npm already containing node_modules.

If future templates got larger -- maybe a template with a lot of static resources -- I could see this issue coming up again.

— Reply to this email directly, view it on GitHub https://github.com/pushkin-consortium/pushkin/issues/322#issuecomment- 2035517339, or unsubscribe https://github.com/notifications/unsubscribe- auth/ABOVVYZCM3XLDWPEY43HNUDY3RQXZAVCNFSM6AAAAABFVYY7D6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZVGUYTOMZTHE. You are receiving this because you commented.Message ID: <pushkin- @.***>

pushkin-consortium / pushkin

Switch to yauzl for unzipping template files #322