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

Changed trim stats to stream and added test #615

Merged

Conversation

ethriel3695
Copy link
Contributor

@ethriel3695 ethriel3695 commented Jul 26, 2022

Context

This PR aims to address the issue that we are seeing for some of our customers, where the Webpack preview stats file is too large to parse through for the TurboSnap process.

In Storybook, we already write a stream for the stats file so it makes sense that we could read a stream and get the file contents accordingly.
Here is the file reference in Storybook
https://github.com/storybookjs/storybook/blob/next/code/lib/core-server/src/utils/output-stats.ts

Changes

I added a function to process the reading of the stats file so I could add a test for that functionality
Added a jest test to verify that the readStatsFile function is returning an Array of objects as expected with an id property.

Added a new library that we also use in Storybook
https://github.com/discoveryjs/json-ext#stringifystreamvalue-replacer-space

Testing

  1. I ran npx chromatic trim-stats-file on my branch and saved the file locally in a different location
  2. I then pushed my latest changes
  3. I switched back to my main branch, pulled the latest code, and ran yarn to update dependencies
  4. Then, I ran npx chromatic trim-stats-file again and compared the file with the other file I had saved locally and which was built and trimmed off my branch.

The image shows an exact match other than white space
Screen Shot 2022-07-27 at 9 30 27 PM

@linear
Copy link

linear bot commented Jul 26, 2022

AP-1905 Turbosnap breaks with large stats file

Proposed solution in comments is to stream JSON in the trim-stats-file.

Original title: Investigate modifying the way we read the stats file in the chromatic-cli

monday.com is experiencing an issue where TurboSnap fails because their stats file is too large

Here is the CLI output that they sent me

✖ Failed to publish your built Storybook
Could not retrieve dependent story files.
Cannot create a string longer than 0x1fffffe8 characters
→ View the full stacktrace below

If you need help, please chat with **** at https://www.chromatic.com/docs/cli for the fastest response.
You can also email the team at [email protected] if chat is not an option.

Please provide **** with the above CLI output and the following info:
{
  "timestamp": "2022-04-05T11:15:55.039Z",
  "sessionId": "c4e5101c-063d-43c2-8625-7cc28f16811f",
  "gitVersion": "2.11.0",
  "nodePlatform": "linux",
  "nodeVersion": "16.14.0",
  "packageName": "chromatic",
  "packageVersion": "6.5.3",
  "storybook": {
    "addons": [
      {
        "name": "a11y",
        "packageName": "@storybook/addon-a11y",
        "packageVersion": "6.4.9"
      },
      {
        "name": "essentials",
        "packageName": "@storybook/addon-essentials",
        "packageVersion": "6.4.9"
      },
      {
        "name": "links",
        "packageName": "@storybook/addon-links",
        "packageVersion": "6.4.9"
      },
      {
        "name": "storyshots",
        "packageName": "@storybook/addon-storyshots",
        "packageVersion": "6.4.9"
      }
    ],
    "staticDir": [
      "./public",
      ".msw/public"
    ],
    "viewLayer": "react",
    "version": "6.4.9"
  },
  "flags": {
    "onlyChanged": "",
    "branchName": "fix/tombo/chromatic-diff2",
    "exitOnceUploaded": "",
    "exitZeroOnChanges": "",
    "projectToken": [],
    "outputDir": [],
    "storybookBuildDir": [],
    "externals": [],
    "untraced": [],
    "interactive": true,
    "appCode": []
  },
  "buildScript": "WITH_SB_BABEL=true NODE_OPTIONS=--max_old_space_size=4096 build-storybook -s ./public,.msw/public",
  "spawnParams": {
    "client": "npm",
    "clientVersion": "8.3.1",
    "nodeVersion": "v16.14.0",
    "platform": "linux",
    "command": "npm",
    "clientArgs": [
      "run",
      "--silent"
    ],
    "scriptArgs": [
      "build-storybook",
      "--",
      "--output-dir",
      "/tmp/chromatic--424-sY7rEKLVBDzN",
      "--webpack-stats-json",
      "/tmp/chromatic--424-sY7rEKLVBDzN"
    ]
  },
  "exitCode": 0,
  "exitCodeKey": "OK",
  "errorType": "Error",
  "errorMessage": "✖ Failed to publish your built Storybook",
  "build": {
    "id": "624c1c19685ee5003ad5c6ac",
    "number": 14205
  }
}

Error: ✖ Failed to publish your built Storybook
Could not retrieve dependent story files.
Cannot create a string longer than 0x1fffffe8 characters
    at Object.slice (node:buffer:593:37)
    at Buffer.toString (node:buffer:811:14)
    at stripBom (/codefresh/volume/dapulse/node_modules/chromatic/bin/main.cjs:3:626226)
    at Object. (/codefresh/volume/dapulse/node_modules/chromatic/bin/main.cjs:3:625424)

Specifically, this line Cannot create a string longer than 0x1fffffe8 characters

Upon further research, I discovered that their stats file is 523 MB, which is larger than the 512 MB maximum for the string data type

I had Tom Bogin, from Monday, attempt to run the trim-stats-file and that did not work either because the file is too large

Tom proposed that maybe we could make the trim stats file stream the JSON like SB does when it writes it in the first place?

@ghengeveld What do you think about this?

Our stats file, for comparison, is only about 77 MB

Monday's stat file is really large and I'm not convinced that there isn't a configuration issue on their end

Possible Investigative Routes

However, I do think that it would be worth looking into if we can stream the JSON via the trim stats file process

Alternatively, we could trim the stats file when we build Storybook and be proactive about it

@ethriel3695 ethriel3695 marked this pull request as ready for review July 28, 2022 03:30
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

Awesome! LGTM @ethriel3695

package.json Outdated Show resolved Hide resolved
Copy link
Member

@ghengeveld ghengeveld left a comment

Choose a reason for hiding this comment

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

Nice, I like it!

@ethriel3695 ethriel3695 merged commit 8138237 into main Jul 28, 2022
@ethriel3695 ethriel3695 deleted the reuben/ap-1905-turbosnap-breaks-with-large-stats-file branch July 28, 2022 16:33
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.

4 participants