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

Yarn Berry Support #166

Closed
jnlsn opened this issue Jul 23, 2020 · 28 comments · Fixed by #393
Closed

Yarn Berry Support #166

jnlsn opened this issue Jul 23, 2020 · 28 comments · Fixed by #393
Assignees
Labels
bug Classification: Something isn't working small Estimate: <= 1 day (> 2 hours)

Comments

@jnlsn
Copy link

jnlsn commented Jul 23, 2020

The Chromatic CLI seems to have issues running in a Yarn v2 project. Whenever we run yarn chromatic we get this error:

UnhandledPromiseRejectionWarning: TypeError: loggly.createClient is not a function

Wondering if this is a known issue with Yarn 2 or something else.

┆Issue is synchronized with this Asana task by Unito

@tmeasday
Copy link
Member

That seems odd, I think we are using https://github.com/loggly/node-loggly-bulk in a fairly straightforward and still the currently recommended way.

I don't know too much about Yarn Berry, do have any idea about how to debug it?

@kylesuss
Copy link
Contributor

kylesuss commented Jul 24, 2020

I had another report of this come through on support today. Yarn v2 user as well, same error.

@ghengeveld
Copy link
Member

ghengeveld commented Jul 27, 2020

I suspect this is indeed broken, but only gets surfaced by Yarn 2. We should handle rejected promises globally as a catch-all:

process.on('unhandledRejection', error => ...);

@ghengeveld
Copy link
Member

I've released an alpha version which adds a promise rejection handler and replaces the default loggly import with a named createClient import. Hopefully that'll resolve the issue. @jayarnielsen could you verify? You can install it via yarn add chromatic@alpha or just use npx chromatic@alpha.

@jnlsn
Copy link
Author

jnlsn commented Jul 27, 2020

Hi @ghengeveld 👋 Thanks for getting back to me so quickly. I'm now getting this error Unhandled promise rejection: TypeError: createClient is not a function on chromatic 5.1.0-alpha.3

@ghengeveld
Copy link
Member

Hi @jayarnielsen, I was able to reproduce and fix the issue with Yarn 2. I've published another alpha which should resolve the issue. Could you verify? Install with yarn add chromatic@alpha or run with npx chromatic@alpha. Thanks!

@ghengeveld
Copy link
Member

Probably you're going to run into "Could not find a supported Storybook viewlayer package" now. I'm looking into it.

@ghengeveld
Copy link
Member

@jayarnielsen I've tried getting Yarn 2 to work for a couple hours but it's currently failing at build-storybook which fails with Module not found: Error: Can't resolve 'babel-loader'

I'm out of ideas right now. I did find this supposed workaround, which might work:
https://github.com/yarnpkg/berry/blob/master/packages/plugin-node-modules/README.md

@jnlsn
Copy link
Author

jnlsn commented Jul 28, 2020

Maybe babel-loader need to be a dependency and not a devDependency? I'm still wrapping my head around yarn 2's strict dependency requirements.

@jnlsn
Copy link
Author

jnlsn commented Jul 28, 2020

This resolver is definitely problematic with yarn's package structure. I wonder if you could parse the project's package.json file instead?

   resolve(process.cwd(), `@storybook/${name}/package.json`, (err, results) => {
      if (err) {
        rej(err);
      } else {
        res(results);
      }
    });

@ghengeveld
Copy link
Member

I fixed that by using require.resolve as recommended in the Yarn doc. See #168

@tmeasday
Copy link
Member

@ghengeveld is this issue fixed?

@tmeasday tmeasday added the bug Classification: Something isn't working label Sep 25, 2020
@tmeasday tmeasday added this to the Q4 Backlog milestone Sep 25, 2020
@tmeasday tmeasday added planned Tracking: Issue is tracked internally small Estimate: <= 1 day (> 2 hours) labels Oct 2, 2020
@ghengeveld
Copy link
Member

There's a PR pending for this: #168

@serenabux
Copy link

Hi there, thanks for working on this! Do you have any estimate of when this PR will be merged and chromatic will work with yarn 2?

@larrifax
Copy link

I had trouble getting chromatic to work in a yarn 2 repo, as it was failing to run the correct build script. Applying the following patch fixed the problem for me:

diff --git a/bin/tasks/build.js b/bin/tasks/build.js
index 1b000848db691ba729a9cce055fe34ae0d68014d..78cae42a284029082602673baef3936ee1c821ac 100644
--- a/bin/tasks/build.js
+++ b/bin/tasks/build.js
@@ -27,10 +27,10 @@ export const setSpawnParams = ctx => {
   // Based on https://github.com/mysticatea/npm-run-all/blob/52eaf86242ba408dedd015f53ca7ca368f25a026/lib/run-task.js#L156-L174
   const npmExecPath = process.env.npm_execpath;
   const isJsPath = typeof npmExecPath === 'string' && /\.m?js/.test(path.extname(npmExecPath));
-  const isYarn = npmExecPath && path.basename(npmExecPath) === 'yarn.js';
+  const isYarn = npmExecPath && path.basename(npmExecPath).startsWith('yarn');
   ctx.spawnParams = {
     command: isJsPath ? process.execPath : npmExecPath || 'npm',
-    clientArgs: [isJsPath ? npmExecPath : '', isYarn ? '' : 'run', '--silent'].filter(Boolean),
+    clientArgs: [isJsPath ? npmExecPath : '', isYarn ? '' : 'run', isYarn ? '' : '--silent'].filter(Boolean),
     scriptArgs: [
       ctx.options.buildScriptName,
       isYarn ? '' : '--',

I assume this breaks yarn 1 compatibility though, so it may need some modifications.

I did not run into the OP's issue, so that may have fixed itself?

@jnlsn
Copy link
Author

jnlsn commented Nov 17, 2020

The original error I posted about seems to have sorted itself out. However I'm still running into issues using chromatic with a yarn 2 repo. It seems to have issues finding the version of storybook, even when I use the env variable CHROMATIC_STORYBOOK_VERSION. While attempting to get this working with the github action, I keep running into this error:

TypeError: ✖ Failed to build Storybook
Invalid Version: undefined

My workflow looks like this:

- name: Run Chromatic
  uses: chromaui/action@v1
  with:
    token: ${{ secrets.GH_TOKEN }}
    projectToken: ${{ secrets.CHROMATIC_PROJECT_TOKEN }}
    exitZeroOnChanges: true
    allowConsoleErrors: true
    buildScriptName: "build:storybook"
  env:
    CHROMATIC_STORYBOOK_VERSION: react@6.0.28

Here's the output I'm getting from the action:

 {
  "timestamp": "2020-11-17T19:10:10.270Z",
  "sessionId": "8ff6ec28-8909-4de3-918c-c1436647afb4",
  "gitVersion": "2.29.2",
  "nodePlatform": "linux",
  "nodeVersion": "12.13.1",
  "packageName": "chromatic",
  "packageVersion": "5.4.0",
  "storybook": {
    "viewLayer": "react",
    "addons": []
  },
  "flags": {
    "projectToken": "***",
    "buildScriptName": "build:storybook",
    "fromCI": true,
    "interactive": false,
    "exitZeroOnChanges": true,
    "exitOnceUploaded": false,
    "allowConsoleErrors": true
  },
  "buildScript": "build-storybook -c .storybook -o ./build",
  "errorType": "TypeError",
  "errorMessage": "✖ Failed to build Storybook"
}

@tmeasday tmeasday added the P0 label Jan 28, 2021
@tmeasday
Copy link
Member

Is this still an issue for folks?

@tmeasday tmeasday removed this from the Q2 Improvements milestone Jun 30, 2021
@tmeasday tmeasday removed the planned Tracking: Issue is tracked internally label Jun 30, 2021
@tmeasday
Copy link
Member

Going to close for now, let us know if there is still a problem!

@darleendenno
Copy link

@zol zol reopened this Aug 11, 2021
@tmeasday tmeasday added this to the Q3 Improvements milestone Aug 12, 2021
@ndelangen
Copy link
Member

ndelangen commented Aug 12, 2021

I'm trying to reproduce this.

I made a new storybook repoduction with pnp enabled.
https://github.com/ndelangen/storybook-pnp-repro

I immediately ran into a problem getting storybook to run.
I applied a hot hack to my .pnp.cjs file and got it to run.
https://discord.com/channels/486522875931656193/490070912448724992/875332132535410688

Next I installed chromatic, and my .pnp.cjs file got overwritten, breaking storybook again. I applied my fix again, and got storybook to run again.

Now I try and run the chromatic command, but this fails.
Seems yarn pnp and the esm-package are fighting and neither wins.

TypeError: Ko._resolveFilename is not a function
    at wu (/Users/dev/.yarn/berry/cache/esm-npm-3.2.25-762b3ebd40-8.zip/node_modules/esm/esm.js:1:226797)
    at Eu (/Users/dev/.yarn/berry/cache/esm-npm-3.2.25-762b3ebd40-8.zip/node_modules/esm/esm.js:1:227999)
    at Module.<anonymous> (/Users/dev/.yarn/berry/cache/esm-npm-3.2.25-762b3ebd40-8.zip/node_modules/esm/esm.js:1:295976)
    at n (/Users/dev/.yarn/berry/cache/esm-npm-3.2.25-762b3ebd40-8.zip/node_modules/esm/esm.js:1:279589)
    at Object.<anonymous> (/Users/dev/.yarn/berry/cache/chromatic-npm-5.9.2-9561fc1d25-8.zip/node_modules/chromatic/bin/register.js:4:23)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.external_module_.Module._load (/Users/dev/Projects/Chroma/repro-pnp/.pnp.cjs:27681:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:76:12)

@ndelangen
Copy link
Member

@ghengeveld You recently made a package @storybook/expect which is a fully pre-bunded package. I wonder if we should pre-bundle this CLI and the action into 2 files in a build process. WDYT?

@nujhong
Copy link

nujhong commented Aug 14, 2021

Currently running to the same issue.

It seems that this might be supported in Yarn 3.x in the future but it's still in draft yarnpkg/berry#2161

@mayteio
Copy link

mayteio commented Aug 18, 2021

I am also getting the following error;

TypeError: Ko._resolveFilename is not a function

We run this in CircleCI. In the meantime, we've got the following step in the job that switches to using the node-modules linker for pnp, which allows the job to run successfully:

  storybook_www:
    steps:
      - checkout
      - run:
          name: Change yarn pnp linker to node-modules
          command: sed -i '' 's/pnp/node-modules/' .yarnrc.yml
      - run: yarn install
      - run: npx chromatic ...

@tobilen
Copy link

tobilen commented Aug 20, 2021

I am also getting the following error;

TypeError: Ko._resolveFilename is not a function

We run this in CircleCI. In the meantime, we've got the following step in the job that switches to using the node-modules linker for pnp, which allows the job to run successfully:

  storybook_www:
    steps:
      - checkout
      - run:
          name: Change yarn pnp linker to node-modules
          command: sed -i '' 's/pnp/node-modules/' .yarnrc.yml
      - run: yarn install
      - run: npx chromatic ...

sed -i '0,/pnp/{s/pnp/node-modules/}' .yarnrc.yml to avoid changing other config values like pnpIgnorePatterns or pnpMode. But yeah, otherwise this works. It massively slows down our CI though since this opts us out of zero installs :(

@zol
Copy link
Member

zol commented Aug 20, 2021

It massively slows down our CI though since this opts us out of zero installs :(

Not ideal, hopefully we can figure out a better fix.

@mayteio
Copy link

mayteio commented Aug 21, 2021

@zol same, we split chromatic into its own job that runs parallel so our other jobs can run with zero install. Best we could figure out until this is fixed.

@ghengeveld
Copy link
Member

A workaround for Yarn 2 / PnP was outlined by @cmbirk here: https://github.com/CrossroadsCX/action/blob/afe74f34a1608ecb8571b65163a7695ecf1af0d4/README.md#yarn-2-support

We still plan to have native support for Yarn 2 / PnP, but this might help anyone who needs a quick workaround.

@domarmstrong
Copy link

A better workaround for anyone coming across this #578 (comment). No need to change the node linker or reinstall the dependencies. It then works fine with pnp zero installs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Classification: Something isn't working small Estimate: <= 1 day (> 2 hours)
Projects
None yet