Skip to content

14.3.3 causes crashes in Storybook #681

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

Open
avendiart opened this issue Nov 29, 2021 · 5 comments · May be fixed by #883
Open

14.3.3 causes crashes in Storybook #681

avendiart opened this issue Nov 29, 2021 · 5 comments · May be fixed by #883

Comments

@avendiart
Copy link

Reproduction repository: https://github.com/avendiart/storybook-issue, for more context please see storybookjs/storybook#16827, cross-posting it here for reference.

@armandabric
Copy link
Collaborator

armandabric commented Dec 1, 2021

I will said it again here: I have no bandwidth to look into this. It's been a few month my OSS time as vanish (sadly).

I'm open to accept more contributor into this project to help maintain it or to move it to new home (see #659)

@tu4mo
Copy link

tu4mo commented Dec 2, 2021

Probably related to #667.

@armandabric
Copy link
Collaborator

Is reverting change of the 14.3.3 could help for now? It will avoid to hurt to many people while we could take some time to investigate

@jasongornall
Copy link

bumping this as I just ran into this problem

@7rulnik
Copy link

7rulnik commented Apr 4, 2025

So it's still a problem and it even got worse and this is why:

it's impossible to override react-element-to-jsx-string because storybook stared to bundle dependencies since storybook v8.4.0 which was released in Oct 31, 2024

I tried to bisect this problem using storybook 8.3 and this repo. So the OOM was introduced in #619 by @Andarist.

A bit later there was one more PR #660

I did not dig deep into how this lib works but as I can see if we revert #619 all tests still pass and OOM is gone

/* @flow */
import * as React from 'react';

function safeSortObject(value: any, seen: WeakSet<any>): any {
  // return non-object value as is
  if (value === null || typeof value !== 'object') {
    return value;
  }

  // return date, regexp and react element values as is
  if (
    value instanceof Date ||
-    value instanceof RegExp ||
-    React.isValidElement(value)
+    value instanceof RegExp
  ) {
    return value;
  }

  seen.add(value);

  // make a copy of array with each item passed through the sorting algorithm
  if (Array.isArray(value)) {
    return value.map(v => safeSortObject(v, seen));
  }

  // make a copy of object with key sorted
  return Object.keys(value)
    .sort()
    .reduce((result, key) => {
      if (key === '_owner') {
        return result;
      }
      if (key === 'current' || seen.has(value[key])) {
        // eslint-disable-next-line no-param-reassign
        result[key] = '[Circular]';
      } else {
        // eslint-disable-next-line no-param-reassign
        result[key] = safeSortObject(value[key], seen);
      }
      return result;
    }, {});
}

export default function sortObject(value: any): any {
  return safeSortObject(value, new WeakSet());
}

@Andarist I know that it was a long time ago (almost 4 years) but maybe you could see why it triggers OOM or do we really need this React.isValidElement check?

If it's alright to remove this check could we release this fix as 15.0.1 and 17.0.1?

cc @armandabric

@7rulnik 7rulnik linked a pull request Apr 13, 2025 that will close this issue
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 a pull request may close this issue.

5 participants