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

Use @antfu/ni to support pnpm for Storybook build #789

Merged
merged 14 commits into from
Sep 1, 2023

Conversation

ghengeveld
Copy link
Member

@ghengeveld ghengeveld commented Jul 18, 2023

Rather than rolling our own package manager script runner logic, use @antfu/ni to handle it for us. This adds support for pnpm and Bun, since ni supports those too.

Note: an enhancement would be to upgrade Execa to the same version ni uses, but I did not go down that road yet because it upgraded to ESM and therefore isn't compatible with our current Jest config. I'd love to fix that but it would be a larger undertaking.

@linear
Copy link

linear bot commented Jul 18, 2023

AP-2652 Support pnpm in our CLI

What

When running patch builds we run yarn or npm on the user's behalf. This ticket pertains to running pnpm if the user is using it.

Why

Users who are running pnpm don't want us to run yarn (the default it seems) for them.

How

We'd probably just need to replace or rewrite the yarn-or-npm library we use and shell out to pnpm.

Copy link
Member

@thafryer thafryer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Largely looks good to me. Few notes to help improve understanding.

Also, looking at the larger picturr of pnpm support, shouldn't we also add the pnpm lockfile as file that TurboSnap bails on?

node-src/tasks/build.test.ts Show resolved Hide resolved
node-src/tasks/build.ts Outdated Show resolved Hide resolved
@ghengeveld ghengeveld force-pushed the ghengeveld/ap-2652-support-pnpm-in-our-cli branch from fac43da to 8a4df55 Compare July 20, 2023 12:40
Copy link
Member

@thafryer thafryer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We'll follow up with PNPM + TurboSnap.

@ghengeveld
Copy link
Member Author

ghengeveld commented Jul 21, 2023

This has been released as 7.0.0-canary.0 available under the canary tag (chromatic@canary) on npm and the chromaui/action-canary GitHub action.

Before we can release this as final, we should ask some customers to give it a try, and do extensive QA on it ourselves. Perhaps the Storybook team could use it.

@glancel
Copy link

glancel commented Aug 25, 2023

Hi! How can we help to test if pnpm works? If I install the canary version the build step of chromatic-cli still uses npm.

@ghengeveld ghengeveld force-pushed the ghengeveld/ap-2652-support-pnpm-in-our-cli branch from 6721fc6 to 54d4958 Compare September 1, 2023 13:25
@ghengeveld ghengeveld merged commit 4952321 into main Sep 1, 2023
20 checks passed
@ghengeveld ghengeveld deleted the ghengeveld/ap-2652-support-pnpm-in-our-cli branch September 1, 2023 13:59
@ghengeveld ghengeveld changed the title Use @antfu/ni to resolve build command Use @antfu/ni support pnpm for storybook build Sep 1, 2023
@ghengeveld ghengeveld changed the title Use @antfu/ni support pnpm for storybook build Use @antfu/ni to support pnpm for storybook build Sep 1, 2023
@ghengeveld ghengeveld changed the title Use @antfu/ni to support pnpm for storybook build Use @antfu/ni to support pnpm for storybook build Sep 1, 2023
@ghengeveld ghengeveld changed the title Use @antfu/ni to support pnpm for storybook build Use @antfu/ni to support pnpm for Storybook build Sep 1, 2023
@ghengeveld
Copy link
Member Author

Released as 7.0.0-next.0.

@ghengeveld
Copy link
Member Author

@glancel Could you give 7.0.0-next.0 a try? If you use GitHub Actions, it's chromaui/action-next. I want to get some 👍 before I ship it as 7.0.0 final.

@glancel
Copy link

glancel commented Sep 4, 2023

@ghengeveld It works great! I haven't tested the Github Action yet but the rest works great without any additional configuration!! Thanks a lot

@glancel
Copy link

glancel commented Sep 5, 2023

@ghengeveld I'm trying to use the github action but I get the chromaui/action-next do not exist:

name: 'Chromatic'
on:
  pull_request:
    types: [ opened, synchronize, reopened ]
    branches:
      - 'main'
env:
  NODE_VERSION: "18.x"
jobs:
  chromatic-deployment:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
        with:
          fetch-depth: 0
      - name: Setup Node ${{ env.NODE_VERSION }}
        uses: actions/setup-node@v3
        with:
          node-version: ${{ env.NODE_VERSION }}
      - name: Setup pnpm
        uses: pnpm/action-setup@v2
        with:
          version: latest
      - name: Install dependencies
        run: pnpm install
      - name: Publish to Chromatic
        uses: chromaui/action-next
        with:
          projectToken: ${{ secrets.CHROMATIC_PROJECT_TOKEN }}

Do you happen to know why this is happening?

@ghengeveld
Copy link
Member Author

I think you needed chromaui/action-next@v1 (with tag). I've already released it as "latest" though, so you can use chromaui/action@v1 now.

@kevinlandsberg
Copy link

LGTM. We'll follow up with PNPM + TurboSnap.

hey @thafryer , is there any status update about this topic? We are actually hitting a blocker by Chromatic not supporting TurboSnap in our current pnpm setup :(
We highly rely on Chromatic and love it, we really don't want to drop it but this way we cannot use it at all..
Or is there some kind of workaround?

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.

None yet

5 participants