gridstack.js icon indicating copy to clipboard operation
gridstack.js copied to clipboard

React : multiple grids crashes

Open PlugNPush opened this issue 11 months ago • 4 comments

Hi,

I wanted to test gridstack for my project and made a very basic grid like this:

export function GridStackPage() {
  const [grid1Options] = useState(() => createGridOptions());
  const [grid2Options] = useState(() => createGridOptions());

  return (
    <div style={{ display: "flex", gap: "20px" }}>
      <div style={{ flex: 1 }}>
        <h3>Grid 1</h3>
        <GridStackProvider initialOptions={grid1Options}>
          <Toolbar gridNumber={1} />
          <GridStackRenderProvider>
            <GridStackRender componentMap={COMPONENT_MAP} />
          </GridStackRenderProvider>
        </GridStackProvider>
      </div>
      <div style={{ flex: 1 }}>
        <h3>Grid 2</h3>
        <GridStackProvider initialOptions={grid2Options}>
          <Toolbar gridNumber={2} />
          <GridStackRenderProvider>
            <GridStackRender componentMap={COMPONENT_MAP} />
          </GridStackRenderProvider>
        </GridStackProvider>
      </div>
    </div>
  );
}

Whenever I want to add a widget in them, it works fine for the last one but crashes for all the ones before (here grid 1).

The reason for the crash is :

Uncaught Error: Widget container not found for id: widget-39vq52659p5
    children grid-stack-render.tsx:55
    GridStackRender grid-stack-render.tsx:47
    React 18
    current grid-stack-render-provider.tsx:35
    renderCB grid-stack-render-provider.tsx:41
    createWidgetDivs utils.ts:128
    addWidget gridstack.ts:470
    addSubGrid grid-stack-provider.tsx:60
    onClick GridStackDemo.tsx:210

It comes from GridStackRenderProvider, in

<GridStackRenderContext.Provider
        value={useMemo(
          () => ({
            getWidgetContainer: (widgetId: string) => {
              return widgetContainersRef.current.get(widgetId) || null;
            },
          }),
          // ! gridStack is required to reinitialize the grid when the options change
          // eslint-disable-next-line react-hooks/exhaustive-deps
          [gridStack]
        )}
      >
        <div ref={containerRef}>{gridStack ? children : null}</div>
      </GridStackRenderContext.Provider>

as widgetContainersRef.current.get(widgetId) unexpectedly returns undefined.

I would appreciate some help here.

PlugNPush avatar Feb 06 '25 17:02 PlugNPush

I don't use React. I assume you are using GridStackProvider and other custom tags from the /rect example ? I would check with those dev who worked on it @Aysnine and #2938

adumesny avatar Feb 06 '25 19:02 adumesny

Yes, I copied the whole lib/ folder from the react example, and basically duplicated the grids from the first provider. I can't get the two of them to work, only the last one does.

Note that only adding a component in a grid is not working, for example, I am able to add a component in the last grid and then drag it over to the one dysfonctionning with no issues.

While trying to bypass this problem, I encountered another: in react there doesn't seem to be a fast forward way to add a component in a subgrid...

PlugNPush avatar Feb 06 '25 20:02 PlugNPush

Hi there. I just updated the react example and submitted the pr. Can you try to provide a minimally reproducible case in this codesandbox using the latest libs?

https://codesandbox.io/p/sandbox/github/Aysnine/gridstack-react/tree/main?file=/src/App.tsx

Aysnine avatar Feb 07 '25 02:02 Aysnine

@Aysnine thanks for being active on that code. I actually wanted to start shipping them like I do for /angular I started looking at the code and was planning to do this weekend... I wanted to match better what I did for angular wrapper (extending WidgetItem to have similar to 'selector' + 'input' which are angular centric but conceptually the same as name + props you have, rathern than JSON string in content. I'll have to think about having a custom data T type, or changing 'content'

adumesny avatar Feb 07 '25 18:02 adumesny

I know exact what happened, renderCB is a static method of class GridStack.What will happend if we modify it? The last override wins, that why multiple grids will crash. see the react grid-stack-render-provider source code

// https://github.com/gridstack/gridstack.js/blob/master/react/lib/grid-stack-render-provider.tsx#L34
GridStack.renderCB = renderCBFn;

But if u see the source code, it was design for that usage, see the source code. It seems that there are lots of that code style in the GridStack class.

I used OOP in decades ago which is not my favorite and unfamiliar now ,But I think it a bad practise.

export class GridStack {
  /**
   * callback to create the content of widgets so the app can control how to store and restore it
   * By default this lib will do 'el.textContent = w.content' forcing text only support for avoiding potential XSS issues.
   */
  // https://github.com/gridstack/gridstack.js/blob/master/src/gridstack.ts#L184-L189
  public static renderCB?: RenderFcn = (el: HTMLElement, w: GridStackNode) => { if (el && w?.content) el.textContent = w.content; };

tearf001 avatar Jul 28 '25 16:07 tearf001

@tearf001 the reason it is static is that (along with other CB I heavely use in angular wrapper to create right component, and is NEVER grid specific) is that you don't necessarly have control of grids. user can create/delete them dynamically (see nested grid samples) or comes from JSON reading from backend, so you don't have a fix set fo grid and handle of them.

this has been working fine for years on Angular and standalone. Just need to understand how to make it work naturally with React, which I do not use.

GridStack.addRemoveCB = gsCreateNgComponents

export function gsCreateNgComponents(host: GridCompHTMLElement | HTMLElement, n: NgGridStackNode, add: boolean, isGrid: boolean): HTMLElement | undefined {
...
      // define what type of component to create as child, OR you can do it GridstackItemComponent template, but this is more generic
      const selector = n.selector;
      const type = selector ? GridstackComponent.selectorToType[selector] : undefined;
      if (type) {
...

adumesny avatar Jul 28 '25 16:07 adumesny

Thanks for your reply, I know the design, that is React wrapper problem. the renderCB should be stateless.

tearf001 avatar Jul 28 '25 16:07 tearf001

yes, and also I would not use 'content' for creating the right widget type. this was historically done for any html content (which has security issues, hence I only support text out of the box now) but custom fields on GridStackWidget like I do for angular wrapper ('selector' being the main one), or id lookup, or....

/** extends to store Ng Component selector, instead/inAddition to content */
export interface NgGridStackWidget extends GridStackWidget {
  /** Angular tag selector for this component to create at runtime */
  selector?: string;
  /** serialized data for the component input fields */
  input?: NgCompInputs;
  /** nested grid options */
  subGridOpts?: NgGridStackOptions;
}

adumesny avatar Jul 28 '25 17:07 adumesny

Yes, I read lots of historical codes, that're great works over years. The react wrapper(grid-stack-render-provider) each uses a local map for the widgets in its context, any override will lead to memory mess. I am refactoring it to make it work ,first in my project. Thank you.

tearf001 avatar Jul 28 '25 17:07 tearf001

if you can take a look at the pending review @Aysnine has done as I need more eyes on React wrapper and make it official... I jsut don't have the expertise on that framework. So I'm relying on you guys and hope to guide it to match what I did for Angualr (which is being heavily used now)

adumesny avatar Jul 28 '25 18:07 adumesny

I 've made a pr to fix this issue.

tearf001 avatar Jul 28 '25 21:07 tearf001