xyflow / xyflow

React Flow | Svelte Flow - Powerful open source libraries for building node-based UIs with React (https://reactflow.dev) or Svelte (https://svelteflow.dev). Ready out-of-the-box and infinitely customizable.
https://xyflow.com
MIT License
22.07k stars 1.46k forks source link

Library breaks when trying to create an Edge from a Handle that is generated programmatically #805

Closed StefanoSega closed 3 years ago

StefanoSega commented 3 years ago

Hi guys! First of all, thanks for the amazing library that React Flow is.

I've a Custom Node where I need to programmatically generate the Source Handles based on a list stored in data; the list is named groups and contains strings like '1', '2', ... id is the Node Id.

const outputHandles = groups.map((group: string, outputIdx: number) => {
    const handleId = `${id}|${outputIdx}`;
    const leftPos = `${((100 / (groups.length + 1)) * (outputIdx + 1))}%`;

    return <Handle
      type="source"
      position={Position.Bottom}
      id={handleId}
      key={handleId}
      style={{ left: leftPos, background: getGroupColor(group) }}
    />;
  });

The Node is generated correctly showing an handle for each group, but when I try to drag a Edge out of the Handle I get this error:

scheduler.development.js:178 Uncaught TypeError: Cannot read property 'find' of null
    at ConnectionLine (cjs.js:12)
    at renderWithHooks (react-dom.development.js:14803)
    at updateFunctionComponent (react-dom.development.js:17034)
    at beginWork (react-dom.development.js:18610)
    at HTMLUnknownElement.callCallback (react-dom.development.js:188)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:237)
    at invokeGuardedCallback (react-dom.development.js:292)
    at beginWork$1 (react-dom.development.js:23203)
    at performUnitOfWork (react-dom.development.js:22154)
    at workLoopSync (react-dom.development.js:22130)
    at performSyncWorkOnRoot (react-dom.development.js:21756)
    at react-dom.development.js:11089
    at unstable_runWithPriority (scheduler.development.js:653)
    at runWithPriority$1 (react-dom.development.js:11039)
    at flushSyncCallbackQueueImpl (react-dom.development.js:11084)
    at flushSyncCallbackQueue (react-dom.development.js:11072)
    at flushPassiveEffectsImpl (react-dom.development.js:22883)
    at unstable_runWithPriority (scheduler.development.js:653)
    at runWithPriority$1 (react-dom.development.js:11039)
    at flushPassiveEffects (react-dom.development.js:22820)
    at react-dom.development.js:22699
    at workLoop (scheduler.development.js:597)
    at flushWork (scheduler.development.js:552)
    at MessagePort.performWorkUntilDeadline (scheduler.development.js:164)
RickeyWard commented 3 years ago

I'm using dynamic handles with no issues, connection lines find the handles by their IDs and types.

if you're getting Cannot read property 'find' of null in ConnectionLine then thats here

const sourceHandle = handleId
    ? sourceNode.__rf.handleBounds[connectionHandleType].find((d: HandleElement) => d.id === handleId)
    : sourceNode.__rf.handleBounds[connectionHandleType][0];

handleBounds is not being updated, this is set when updateNodeDimensions or batchUpdateNodeDimensions are called, which are an action on the main store. batchupdate is called when the window resize, updateNode is called when any node's id, hidden, sourcePosition, or targetPosition changes. which is handled by the wrapNode component. There's possibly some memo-isation happening preventing this from getting called for you. -OR- It's also possible that this is being called and the getHandleBoundsByHandleType function is not finding any elements when it's being called. it looks for any html element under the node parent that has the class source or target respectively. The only way for it to return null is for that list to be empty or null.

  const handles = nodeElement.querySelectorAll(selector);
  if (!handles || !handles.length) {
    return null;
  }

maybe hack in some extra renders on your node to see if that magically fixes it, then we can figure out why.

StefanoSega commented 3 years ago

Hi @RickeyWard , thanks a lot for the support! At the moment I did a workaround in which I always generate e.g. 5 Handles and if I've to show only 2 of them I set isConnectable to False and visibility: hidden to all the others, and when the handles change I remove the edges that are orphans; this way works fine. Once I'll have some time I'll investigate in it.

moklick commented 3 years ago

Thanks @RickeyWard!

I also think that the problem here is that the nodes are not re-initialized. Respectively the nodes are getting initialized without handles or with a different number and then we don't update them.

It would be helpful to have a way to update the node dimensions programmatically.

RickeyWard commented 3 years ago

A way to update the node dimensions programmatically sounds like a great idea, what I'm having trouble identifying is why I can programmatically create handles with no issues and @StefanoSega can't

I have a node that starts with no outbound edges, and has a button that adds a new one. they are stored in the data attribute and are mapped virtually identically to the OPs map. I allow editing in a side pane as well, and when the options are updated the handles also work as expected.

burberger commented 3 years ago

I have a very similar problem to this. For my use case, users have the ability to edit the identifiers associated with handles, which causes the calculated pin positions to fall out of sync until the node is resized or fully recalculated because it's fallen out of view.

I'm not sure what the best solution is, my current thought is to add a MutationObserver to the root div of the node which invokes updateNodeDimensions whenever the contents mutate, but it's not obvious to me if there's a simpler way to cause the existing effects in wrapNode.tsx to fire instead.

Jaspooky commented 3 years ago

I ran into this issue today, after some experimentation I found what was triggering this, and the potential answer to

what I'm having trouble identifying is why I can programmatically create handles with no issues and @StefanoSega can't

I have a feature for adding custom labels onto nodes as part of the UI. What I found was that if adding/removing dynamic handles alters the width of the element (i.e. the calculated width based on handle count grows/shrinks past the width of the custom label), then dynamic ports work fine, however if adding the dynamic handle does not alter the width of the element, then it crashes out.

This can be observed in image posted as part of https://github.com/wbkd/react-flow/issues/811 - in the example given, adding the handle does not affect the size of the element which leads to the issue at hand. So it may be that @RickeyWard's implementation causes the component's width to change whereas @StefanoSega may have fixed size components?

RickeyWard commented 3 years ago

My use case doesn't cause the width to change but it does cause the height to change. Maybe that's it this probably needs some attention to be more deterministic.

moklick commented 3 years ago

For clarification: We are updating the node internals whenever a node mounts or changes the id, isHidden, sourcePosition or targetPosition option. More over we are observing every node with a ResizeObserver. So when you change the width or height of a node we are also updating it. @burberger came up with the idea to add a MutationOberserver too in #859 but it affects the performance. Now the idea is to create a useUpdateNode hook or an updateNode action to update a specific node.

burberger commented 3 years ago

I haven't had time to push that up, but I have it working in my app. I'll post an updated PR that uses the hook instead in the next day or two.

moklick commented 3 years ago

In v9.1.0 we added a useUpdateNodeInternals hook that can be used to update the node internals programatically.

Usage:

import { useUpdateNodeInternals } from 'react-flow-renderer';

// ....

const updateNodeInternals = useUpdateNodeInternals();

// somewhere in your app
updateNodeInternals('node-id');

Does this solve your issue?

RickeyWard commented 3 years ago

It already worked in my particular case because of resizing, but I added this in also and it still behaves as expected for me.

moklick commented 3 years ago

I will close this issue since the useUpdateNodeInternals hook should solve it. If the issue still remains feel free to reopen it.

Reflex-Gravity commented 3 years ago

When should I call the useUpdateNodeInternals hook? Currently, the below code is not fixing it. As I'm getting the above-mentioned error when I try to create an Edge.

In my implementation, I have a textfield that asks for the no. of handles in a Custom Node component. So, here in the below code, I call the updateNodeInternals when the count changes.

const ChatTriggerContents = ({ widgetData, handleTextChange }) => {
    const updateNodeInternals = useUpdateNodeInternals()

    useEffect(() => {
        Array.from({ length: widgetData.data.transitionCount }).forEach((_trans, _transIndex) => {
            updateNodeInternals(`handle_${_transIndex}`)
        })
    }, [widgetData.data.transitionCount, updateNodeInternals])

    return (
        <>
            <Typography className="font-bold p-2 text-center">{widgetData?.data?.action_name}</Typography>

            {Array.from({ length: widgetData?.data?.transitionCount ?? 0 }).map((_trans, _transIndex) => {
                return (
                    <Handle
                        key={`handle_${_transIndex}`}
                        id={`handle_${_transIndex}`}
                        type="source"
                        style={{ left: `${getPos(_transIndex, parseInt(widgetData?.data?.transitionCount, 10))}%` }}
                        position="bottom"
                    />
                )
            })}
        </>
    )
}
RickeyWard commented 3 years ago

When should I call the useUpdateNodeInternals hook? updateNodeInternals(handle_${_transIndex})

updateNodeInternals takes the ID of the node, you are passing the ID of the handle,

Reflex-Gravity commented 3 years ago

When should I call the useUpdateNodeInternals hook? updateNodeInternals(handle_${_transIndex})

updateNodeInternals takes the ID of the node, you are passing the ID of the handle,

Ohh My Bad. Thanks. It works now.

sajaljai commented 2 years ago

Using useUpdateNodeInternals breaks the whole application and gives below error. Our application not using redux. Please suggest how we update internal of a node programatically without using redux.

Uncaught Error: could not find react-redux context value; please ensure the component is wrapped in a Provider at useReduxContext (useReduxContext.js:24) at useStore (useStore.js:20) at useDispatch (useDispatch.js:17) at useStoreActions (index.js:12) at useUpdateNodeInternals (style-inject.es.js:27) at CampaignFlowChart (CampaignFlowChart.js:64) at renderWithHooks (react-dom.development.js:14803) at mountIndeterminateComponent (react-dom.development.js:17482) at beginWork (react-dom.development.js:18596) at HTMLUnknownElement.callCallback (react-dom.development.js:188) at Object.invokeGuardedCallbackDev (react-dom.development.js:237) at invokeGuardedCallback (react-dom.development.js:292) at beginWork$1 (react-dom.development.js:23203) at performUnitOfWork (react-dom.development.js:22154) at workLoopSync (react-dom.development.js:22130) at performSyncWorkOnRoot (react-dom.development.js:21756) at react-dom.development.js:11089 at unstable_runWithPriority (scheduler.development.js:653) at runWithPriority$1 (react-dom.development.js:11039) at flushSyncCallbackQueueImpl (react-dom.development.js:11084) at flushSyncCallbackQueue (react-dom.development.js:11072) at scheduleUpdateOnFiber (react-dom.development.js:21199) at Object.enqueueForceUpdate (react-dom.development.js:12678)

Jaspooky commented 2 years ago

Using useUpdateNodeInternals breaks the whole application and gives below error. Our application not using redux. Please suggest how we update internal of a node programatically without using redux.

Uncaught Error: could not find react-redux context value; please ensure the component is wrapped in a Provider at useReduxContext (useReduxContext.js:24) at useStore (useStore.js:20) at useDispatch (useDispatch.js:17) at useStoreActions (index.js:12) at useUpdateNodeInternals (style-inject.es.js:27) at CampaignFlowChart (CampaignFlowChart.js:64) at renderWithHooks (react-dom.development.js:14803) at mountIndeterminateComponent (react-dom.development.js:17482) at beginWork (react-dom.development.js:18596) at HTMLUnknownElement.callCallback (react-dom.development.js:188) at Object.invokeGuardedCallbackDev (react-dom.development.js:237) at invokeGuardedCallback (react-dom.development.js:292) at beginWork$1 (react-dom.development.js:23203) at performUnitOfWork (react-dom.development.js:22154) at workLoopSync (react-dom.development.js:22130) at performSyncWorkOnRoot (react-dom.development.js:21756) at react-dom.development.js:11089 at unstable_runWithPriority (scheduler.development.js:653) at runWithPriority$1 (react-dom.development.js:11039) at flushSyncCallbackQueueImpl (react-dom.development.js:11084) at flushSyncCallbackQueue (react-dom.development.js:11072) at scheduleUpdateOnFiber (react-dom.development.js:21199) at Object.enqueueForceUpdate (react-dom.development.js:12678)

From the docs

The following hooks can only be used if the component that uses it is a children of a ReactFlowProvider.

Have you included the provider in your component tree as requested by the error?

sajaljai commented 2 years ago

Hi Jaspooky, I missed that provider part in doc, it is working now when implemented in children of provider.

Alexey174 commented 2 years ago

my component with react flow is included in the react flow provider. useUpdateNodeInternals is connected and works without errors. only it does not update the CustomNode. I wanted to ask if I'm doing the right thing - I make changes to the object of the desired CustomNode, update the elements using setElements and then call useUpdateNodeInternals ("id_my_customNode"), for example, useUpdateNodeInternals("2"). at the same time, there are changes in the CustomNode object, but the content of the CustomNode in the DOM tree does not change

Nearoo commented 2 years ago

In v9.1.0 we added a useUpdateNodeInternals hook that can be used to update the node internals programatically.

Works like a charm! Could you also add a simple function that does that? My Node is a class component, and react doesn't allow usage of hooks outside of function components, so I have to do a workaround somewhere or do it at an awkward location. Thanks!

RickeyWard commented 2 years ago

Could you also add a simple function that does that? My Node is a class component, and react doesn't allow usage of hooks outside of function components, so I have to do a workaround somewhere or do it at an awkward location. Thanks!

The api for the library is hook-heavy, slippery slope to start adding reference functions for all the actions and since no non-hook state updates exist in the internal api it would end up being burying a work around in the library instead of the client code.

I'd recommend using functional components when working with react-flow as that's how the library is written but the easiest escape hatch would be a forwardref and an imperative handle. (note that you could always just call the redux dispatch event that performs the update but since that's relying on the state library not changing I strongly recommend against it and always targeting the outward facing api)

Example for creating an imperative api exposing component. (completely untested written in this comment, but I'm pretty sure it's all there.)

function UpdateInterals(props, ref) {
  const updateNodeInternals = useUpdateNodeInternals();
  useImperativeHandle(ref, () => ({
    updateNodeInternals: updateNodeInternals
    }
  }));
  return null;
}
UpdateNodeInterals = forwardRef(UpdateNodeInterals);

Then you can just render this Component in your Node and use the ref to access the object. In fact you could grab all the hook functions you wanted to and add them to the ref.

The ref will expose the object returned by the useImperativeHandle callback, so go to town and have a good time.

class MyComponent extends React.Component {
  constructor(props) {
    super(props);
    this.myRef = React.createRef();
  }

  updateInterals(args) {
     if(this.myRef.current)
     {
       this.myRef.current.updateNodeInternals(args);
     }
  }

  render() {
    return (<div>
       <UpdateNodeInterals  ref={this.myRef}/>
    </div>;)
  }
}
mike-oakley commented 2 years ago

Hi,

I've ran into the dynamic handles issue - i'm adding a new source handle and edge from a UI interaction. I've implemented the updateNodeInternals call in a useEffect in the relevant custom node like so:

useEffect(() => {
    // Whenever the ports change we need to inform React-Flow so that it can
    // recalculate internal state. See https://github.com/wbkd/react-flow/issues/805.
    updateNodeInternals(id);
  }, [id, updateNodeInternals, ports]);

(the ports variable contains the number and location of the node handles)

This fixes the edge not being drawn correctly, however I still get a warning beforehand for couldn't create edge for source handle id - am I invoking the updateNodeInternals callback at the wrong time or is the warning still expected? I tried invoking the update before the node gets updated with the new handle, but this seemed to break the edge being drawn again so I assumed it was wrong.

Thanks!

RickeyWard commented 2 years ago

@CheesyFeet The useEffect on number of outputs is exactly how I use it, works fine. What's more likely is that your handle IDs are not consistent or your edges are being created with the the wrong data, that warning happens when there exists an edge with source, sourcehandle, target, targehandle sets that can't be found in the graph. If you are seeing the handles then they are there, but the IDs possibly don't match.

The code snippet above is fine. updateNodeInternals should happen after the new handle is added. But its important that the node never get rendered without that handle afterwards (unless you've removed all edges that reference it) and also that the handle ID doesn't change.

bogdanAndrushchenko commented 2 years ago

Hi, everyone! I had the same problem as @StefanoSega . I fixed it, downgraded it to version 9.5.4 npm uninstall react-flow-renderer npm install react-flow-renderer@9.5.4