BuilderIO / builder

Drag and drop headless CMS for React, Vue, Svelte, Qwik, and more
https://builder.io
MIT License
6.77k stars 840 forks source link

Critical problem with the <Columns> component when using @builder.io/react-sdk #3219

Open x1024 opened 3 weeks ago

x1024 commented 3 weeks ago

Describe the bug

The Columns component doesn't update its state correctly with the Gen2 Builder.IO SDK. This happens on an empty, brand-new NextJS app, using the latest version of the @builder.io/react-sdk package.

To Reproduce Steps to reproduce the behavior:

  1. Create a new app with create-next-app --typescript, use the default settings.
  2. Integrate the Gen2 SDK: npm install @builder.io/react-sdk and set up a [[page]] route that gets data from your Builder.IO space. For generateStaticParams get a collection of Builder pages and register their URLs. For the page route, get a single page and render it with the <Content /> component, provided by Builder.IO.
  3. Set http://localhost:3000 as the preview URL for the Page model of the project.
  4. Create a page that uses the Columns component
  5. Try to resize the columns by dragging the provided slider.

This is a repo with a commit that reproduces the problem: https://github.com/x1024/builder-bug-demo/tree/bug-present

Expected behavior When the "columns slider" is dragged, the size of the columns needs to dynamically resize as the user is dragging. In reality, the slider does get dragged, and the state of the Builder.IO object does get updated, but the preview on the screen does not. The "Reload preview" button has to manually be pressed in order for the preview to update. I am attaching a video demonstrating both the bugged behavior, in the commit tagged bug-present, as well as the fixed behavior in the commit tagged bug-fixed

Video

https://github.com/BuilderIO/builder/assets/113459/1087f641-937b-490c-b9a0-682132725251

Proposed solution

As far as I can tell, the issue stems from this commit: https://github.com/BuilderIO/builder/commit/31cbe84a98030f94c82697f4186c3c9cab4d7473

The commit changes a few state vlues of the Columns component in the useStore Mitosis helper function. The cols and gutter values are changed - instead of getter functions, they are now values: https://github.com/BuilderIO/builder/commit/31cbe84a98030f94c82697f4186c3c9cab4d7473#diff-a964d50f7f336a25c59e6f9ad90b542e0db0cf9f5924ee3637a9a58a6d31f7d1L35

Screenshot 2024-04-19 at 21 37 45

In React this translates to the cols/gutter being state (via the useState hook) with a default value provided in the component's props:

(the actual code in the npm package is minified, but that's what it resolves to)

const [cols, setCols] = useState(props.cols || [])

This is actually a bug - the generated setCols function is actually never used (and ESLint says as much). The initial value of "cols" is the only value it will ever have during the component's lifetime! When Builder.io users change the size of the component's columns using the visual builder, the rendered result will not change!

In our organization (Dext) we have forked the repo and patched it so that the bug is no longer present.

Here's the commit we ended up with: https://github.com/dext/builder-io/commit/4c3aa87098b8e8bf8eae4c2f4e23ea30e5a43b92

It simply reverts the usage of "state" and goes back to using getter functions. It should be noted: We are only interested in the React use-case and have not tested in any other deployment scenarios. We make no guarantees that this is production-ready.


On a personal note: This was really unexpected and took a lot of time to debug.

samijaber commented 3 weeks ago

Thank you for filing such a detailed bug, along with the source of the error and how to solve it. We will investigate whether we can revert the commit as you suggested, and proceed once we have confirmed the solution.