Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Support Yarn Berry when creating sources zip #945

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mezannic
Copy link
Contributor

@mezannic mezannic commented Aug 29, 2024

Fixes yarn zip -b firefox command on a monorepo with the following file structure
and browser extension dependencies such as "@xxx/foo-package-a@workspace:*".

.
├── apps
│   ├── xxx
│   └── browser-extension   <-- command is executed from here
├── packages
│   ├── foo
│   │   ├── package-a
│   │   └── package-b
│   └── bar
│       ├── xxx

TODO: add source code export test (dependencies install & build) for every supported package manager.
For now, I tested the build on my project with the following shell script.

#!/bin/sh

# Make script stop on errors
set -euo pipefail

# Cleanup previous build artifacts
rm -Rf /tmp/browser-extension-sources ?> /dev/null

# Build from mono-repository
yarn
yarn zip -b firefox

# Extract sources
mkdir -p /tmp/browser-extension-sources
unzip ./.output/browser-extension-1.0.0-sources.zip -d /tmp/browser-extension-sources
cd /tmp/browser-extension-sources

# Build from extracted sources to ensure that the source archive from the mono-repo is OK
yarn # or `yarn install --mode="skip-build"``
yarn zip -b firefox

Copy link

netlify bot commented Aug 29, 2024

Deploy Preview for creative-fairy-df92c4 ready!

Name Link
🔨 Latest commit e48baea
🔍 Latest deploy log https://app.netlify.com/sites/creative-fairy-df92c4/deploys/66fe8dcc02b9890008ead4e3
😎 Deploy Preview https://deploy-preview-945--creative-fairy-df92c4.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@aklinker1 aklinker1 changed the title Fix sources zip fix: Support Yarn Berry when creating sources zip Aug 30, 2024
@aklinker1
Copy link
Collaborator

This is a great PR, thanks for working on it!

@mezannic mezannic force-pushed the fix-sources-zip branch 3 times, most recently from 2862602 to 1ff0b2f Compare September 30, 2024 13:55
@mezannic
Copy link
Contributor Author

I'm a bit stuck on this one.
The modifications I'm proposing are suitable for my use case, but I feel they might not be suitable for others.

Firstly, I systematically download workspace-prefixed dependencies, because I only want to publish the source code linked to my extension, and not the one linked to my other applications. So I have to extract the folder containing my extension source code from the monorepo hosting all my projets and update its package.json to make it work outside of a workspace before publishing it.
If someone wanted to publish everything from their monorepo, packaging workspace-prefixed dependencies would make little sense for them.

Secondly, I introduce into the zip.ts file exceptions related to yarn-specific protocols. There may be a cleaner way to do it.

Thirdly, I didn’t add zipping & unzipping tests for the workspace architecture mentioned in the first post of this pull request. It seems that it would need a bit of reflexion. How to test this behavior on NPM, pnpm, Bun and Yarn while keeping the tests small and compatible with the CI process?

Finally, I now feel that the hooks you recently introduced to the zip command may be a better way to handle some of the modifications I made in this pull request.

@aklinker1 I will mark this pull request as ready, but feel free to leave your thoughts on it here if you feel that it shouldn’t be merged as is. 

Please note that the first commit I made (changing file://./ to file:./) could probably be cherry-picked and merged with another branch.

@mezannic mezannic marked this pull request as ready for review September 30, 2024 14:47
azarbayejani added a commit to azarbayejani/bandcamp-tempo-adjust that referenced this pull request Nov 17, 2024
wxt isn't compatible with yarn berry. See wxt-dev/wxt#945
wxt + yarn was causing problems generating the sources zip for Firefox.

wxt also has problems with pnpm in a workspace environment. Downloading
"private" packages doesn't work if because wxt uses npm pack to pull in
those packages. npm pack doesn't work with the version name that is
given to linked workspace packages: link:{relative-path}. I'll file
an issue with wxt about this later, but for now I've written a hook in
bandcamp-tempo-adjust/wxt.config.ts that regenerates the Firefox sources
zip.

Firefox sources zip generation for discogs-tempo-adjust isn't fixed in this
commit. I'll get to that later.

Also, a lot of references to yarn still exis in README.md. I'll also get
to that later.
@azarbayejani
Copy link

azarbayejani commented Nov 19, 2024

I also had problems using wxt to generate a sources zip when using a pnpm monorepo with a similar structure to you, @mezannic. I ended up following your idea to use the hooks in the zip command to make a sources zip that worked.

Hoping maybe this will help others:

export default defineWxtModule({
  name: 'wxt-monorepo-hooks',
  hooks: {
    'zip:sources:done': async (wxt, sourcesZipPath) => {
      console.log('\x1b[36mℹ\x1b[0m', 'Re-zipping sources...');

      const projectDirName = path.basename(wxt.config.root);

      /*
       * assumes that the workspace is structured like this:
       *
       * packages/
       *   - package-a/
       * apps/
       *   - app-a/ <- wxt.config.root
       */
      const workspaceRoot = path.resolve(wxt.config.root, '../..');

      const zip = new JSZip();

      // zip the workspace root's pnpm-workspace.yaml
      zip.file(
        'pnpm-workspace.yaml',
        fs.readFileSync(path.resolve(workspaceRoot, 'pnpm-workspace.yaml'))
      );

      // zip the workspace root's package.json
      zip.file(
        'package.json',
        fs.readFileSync(path.resolve(workspaceRoot, 'package.json'))
      );

      // get all files in all packages
      const packageFiles = glob.sync('packages/**/*', {
        cwd: workspaceRoot,
        ignore: ['**/node_modules/**/*'],
      });

      // get all files in the current app
      const appFiles = glob.sync(`apps/${projectDirName}/**/*`, {
        cwd: workspaceRoot,
        ignore: ['**/node_modules/**/*'],
      });

      const files = [...packageFiles, ...appFiles];

      // add SOURCE_CODE_REVIEW.md of the current app to the zip root
      zip.file(
        'SOURCE_CODE_REVIEW.md',
        fs.readFileSync(path.resolve(wxt.config.root, 'SOURCE_CODE_REVIEW.md'))
      );

      for (const file of files) {
        const absolutePath = path.resolve(workspaceRoot, file);
        const content = fs.readFileSync(absolutePath);
        zip.file(file, content);
      }

      await new Promise((resolve, reject) => {
        zip
          .generateNodeStream({
            type: 'nodebuffer',
            ...(wxt.config.zip.compressionLevel === 0
              ? { compression: 'STORE' }
              : {
                  compression: 'DEFLATE',
                  compressionOptions: {
                    level: wxt.config.zip.compressionLevel,
                  },
                }),
          })
          .pipe(fs.createWriteStream(sourcesZipPath))
          .on('error', reject)
          .on('close', resolve);
      });
    },
  },
});

@aklinker1
Copy link
Collaborator

@azarbayejani for workspaces where you need to include source code outside the project's root, you can use zip.sourcesRoot. Then you just need to adjust the zipmingludeSources and zip.excludeSources patterns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants