Skip to content
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

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

Closed
StefanoSega opened this issue Dec 27, 2020 · 24 comments

Comments

@StefanoSega
Copy link

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
Copy link
Contributor

RickeyWard commented Dec 28, 2020

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
Copy link
Author

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
Copy link
Member

moklick commented Dec 29, 2020

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
Copy link
Contributor

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
Copy link

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
Copy link

Jaspooky commented Feb 10, 2021

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 #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
Copy link
Contributor

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
Copy link
Member

moklick commented Feb 10, 2021

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
Copy link

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
Copy link
Member

moklick commented Mar 1, 2021

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
Copy link
Contributor

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
Copy link
Member

moklick commented Mar 14, 2021

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

@moklick moklick closed this as completed Mar 14, 2021
@Reflex-Gravity
Copy link

Reflex-Gravity commented Mar 31, 2021

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
Copy link
Contributor

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
Copy link

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
Copy link

sajaljai commented Jul 27, 2021

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
Copy link

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
Copy link

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

@Alexey174
Copy link

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
Copy link

Nearoo commented Sep 2, 2021

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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

@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
Copy link

bogdanAndrushchenko commented Nov 16, 2021

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

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.