Skip to content

Fix recursion with react elements #883

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

Merged
merged 3 commits into from
Apr 25, 2025

Conversation

7rulnik
Copy link
Contributor

@7rulnik 7rulnik commented Apr 13, 2025

It fixes #681

The bug was introduced in #619.

It's not a first time since we encounter this issue, see #307 and especially #312 (comment).

I faced it in storybook and managed to reproduce it using this story:

import { Meta, Story } from "@storybook/react";
import { Buttons, ButtonsProps } from "./Buttons";

export default {
  title: "Components/Buttons",
  component: Buttons,
} as Meta<ButtonsProps>;

export const Default: Story<ButtonsProps> = ({}: ButtonsProps) => (
  <div
    buttons={[
      {
        foo: "bar",
        jsx: <div key="1">Button in array</div>, // this leads to OOM
      },
    ]}
  />
);

but couldn't write test case for it. I tried to reproduce it using code snippet from #312 (comment) with latest react and react-element-to-jsx-string but couldn't for some reasons.

So this is why I decided to add test directly to src/formatter/sortObject.spec.js

It also should fix storybookjs/storybook#16827.

I suggest to backport it to v15 because people mostly use react 18 right now:
image

@7rulnik 7rulnik marked this pull request as ready for review April 13, 2025 18:04
@7rulnik 7rulnik marked this pull request as draft April 14, 2025 07:26
@7rulnik 7rulnik marked this pull request as ready for review April 14, 2025 09:36
@7rulnik
Copy link
Contributor Author

7rulnik commented Apr 16, 2025

@armandabric could you take a look please?

@armandabric
Copy link
Collaborator

I will not merge this I did not have bandwidth to support react 18 and react 19 in parallel. If this is an issue in storybook I advice you to push on storybook side as they use this package from years without helping in any kind.

@ndelangen
Copy link

Hello @armandabric 👋 Storybook maintainer here, Seems You were in touch with @shilman a long time ago, to talk about giving this package a new home (under the storybook org, I assume).

I'd be happy to move the package to the storybookjs org.

@7rulnik
Copy link
Contributor Author

7rulnik commented Apr 22, 2025

Well, I want to clarify that this problem exists in react 19 too. It's not unique for storybook. It could happen in any react environment, see #312 (comment)

@armandabric
Copy link
Collaborator

Hello @armandabric 👋 Storybook maintainer here, Seems You were in touch with @shilman a long time ago, to talk about giving this package a new home (under the storybook org, I assume).

I'd be happy to move the package to the storybookjs org.

Hey! Yes I got in touch with @shilman for that. I'm glad that you are considering to offer a new home for this plugin 😄

Where should we discuss of this? I'm on the storybook discord if needed.

@armandabric
Copy link
Collaborator

Well, I want to clarify that this problem exists in react 19 too. It's not unique for storybook. It could happen in any react environment, see #312 (comment)

Ho! I miss read the issue. We could fix it on the latest branch. I will review the PR

@armandabric armandabric reopened this Apr 23, 2025
@armandabric armandabric self-requested a review April 23, 2025 15:35
Copy link
Collaborator

@armandabric armandabric left a comment

Choose a reason for hiding this comment

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

If not too complicated to write, can you add a test that check that the _owner key is removed from react elements?

@armandabric armandabric merged commit 5679da6 into algolia:master Apr 25, 2025
1 check passed
@armandabric
Copy link
Collaborator

All good, thanks @7rulnik for the fix

@armandabric
Copy link
Collaborator

Fix released on v17.0.1

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.

Out-of-memory in development mode. 14.3.3 causes crashes in Storybook
4 participants