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

Type System and React DOM Prop Mismatches in @xyflow/react Edge Components #631

Open
m0nq opened this issue Jan 9, 2025 · 7 comments
Open

Comments

@m0nq
Copy link

m0nq commented Jan 9, 2025

What platform were you using when you found the bug?

  • React Flow version: @xyflow/react ^12.3.6
  • Browser: Chrome (latest)
  • OS: macOS Sequoia 15.2

Live code example

https://github.com/m0nq/cloud-people/blob/dev/mw/agent-node-states/src/components/sandbox-nodes/automation-edge.tsx

Describe the Bug

Hoping this is just something that I'm missing. Been trying to resolve this for 2 days.

Multiple prop type mismatches between @xyflow/react's TypeScript definitions and React DOM expectations:

  1. Boolean props are typed as boolean but React expects strings:
Error: Received `true` for a non-boolean attribute `animated`.
Error: Received `true` for a non-boolean attribute `selectable`.
Error: Received `true` for a non-boolean attribute `deletable`.
  1. Custom props not being properly handled by React DOM:
Error: React does not recognize the `sourceHandleId` prop on a DOM element.
Error: React does not recognize the `targetHandleId` prop on a DOM element.
Error: React does not recognize the `pathOptions` prop on a DOM element.

Steps to reproduce the bug or issue

  1. Create a custom edge component using BaseEdge
  2. Pass standard edge properties as defined in the type system:
const edge = {
    id: 'edge-1',
    source: 'node-1',
    target: 'node-2',
    animated: true,
    selectable: true,
    deletable: true,
    sourceHandleId: 'handle-1',
    targetHandleId: 'handle-2',
    pathOptions: {}
};

Expected behavior

  1. The type system should match the runtime expectations:
  • If React expects string attributes, props should be typed as strings
  • If props are meant to be boolean, React should handle them as booleans
  1. Custom props should be properly handled or stripped before reaching React DOM

Screenshots or Videos

Screenshot 2025-01-09 at 9 37 14 AM Screenshot 2025-01-09 at 9 38 25 AM Screenshot 2025-01-09 at 9 38 07 AM Screenshot 2025-01-09 at 9 37 55 AM Screenshot 2025-01-09 at 9 37 47 AM Screenshot 2025-01-09 at 9 37 35 AM Screenshot 2025-01-09 at 9 37 24 AM

Additional context

  • These warnings appear in development but don't prevent functionality
  • The type system suggests one usage while runtime requires another
  • Current workaround is to convert booleans to strings: animated: 'true' as any
  • This affects multiple edge properties consistently across the library
  • The issue is more noticeable when creating the first edge connection to a root node
@bcakmakoglu
Copy link
Contributor

bcakmakoglu commented Jan 9, 2025

Are you using Custom Edges and trying to do something like <BaseEdge animated={edge.animated} /> or something?
ReactFlow doesn't set an attribute that's called animated on any element., instead the animated option of edges sets a classname:

// EdgeWrapper.tsx
  return (
    <svg style={{ zIndex }}>
      <g
        className={cc([
          'react-flow__edge',
          `react-flow__edge-${edgeType}`,
          edge.className,
          noPanClassName,
          {
            selected: edge.selected,
            animated: edge.animated, // <------- Edge animated is set here.
            inactive: !isSelectable && !onClick,
            updating: updateHover,
            selectable: isSelectable,
          },
        ])}

Same for the other props you mention, not necessarily a class name but they aren't assigned as attributes to elements.

Edit: I see in the reproduction steps that you mention custom edges.
Could you show us what that Custom Edge looks like?

@m0nq
Copy link
Author

m0nq commented Jan 10, 2025

In the link to my repo you'll see a very basic custom edge.

const AutomationEdge = ({
    id,
    sourceX,
    sourceY,
    sourcePosition,
    targetX,
    targetY,
    targetPosition,
    ...props
}: EdgeProps) => {
    const [edgePath] = getSmoothStepPath({
        sourceX,
        sourceY,
        sourcePosition,
        targetX,
        targetY,
        targetPosition
    });

    return (
        <>
            <BaseEdge id={id} path={edgePath} className="automation-edge" {...props} />
            {/* To add a custom label - also takes any react component as a child */}
            {/* <EdgeLabelRenderer></EdgeLabelRenderer> */}
        </>
    );
};

export default AutomationEdge;

My understanding was that the BaseEdge component consumed EdgeProps which should have the animated property on it 🤔
https://reactflow.dev/api-reference/components/base-edge

@bcakmakoglu
Copy link
Contributor

bcakmakoglu commented Jan 10, 2025

My understanding was that the BaseEdge component consumed EdgeProps which should have the animated property on it 🤔

No it doesn't.
The link you posted even documents the available props for <BaseEdge>.
Although it shows spreading {...props} into it which you want to avoid since that's what is causing your errors here ^^

EdgeProps are the type that is passed to a Custom Edge component like

const CustomEdge = (props: EdgeProps) => {
	// ...
	
	return <BaseEdge ... />
}

@m0nq
Copy link
Author

m0nq commented Jan 10, 2025

But there's an example of spreading the props to the BaseEdge in the page I shared. Can you point me to where it specifies how to animate an edge?

The component gets used internally for all the edges. It can be used inside a custom edge and handles the invisible helper edge and the edge label for you.

import { BaseEdge } from '@xyflow/react';

export function CustomEdge({ sourceX, sourceY, targetX, targetY, ...props }) {
const [edgePath] = getStraightPath({
sourceX,
sourceY,
targetX,
targetY,
});

return <BaseEdge path={edgePath} {...props} />;
}

@bcakmakoglu
Copy link
Contributor

Yeah which is misleading and not accurate in this case. The docs could be updated on that.

You can quite literally see the available props a bit further down though.

image

Can you point me to where it specifies how to animate an edge?

You set edge.animated to true to have it animated.
It's not something you pass to the <BaseEdge> - it is a className that is applied in the <EdgeWrapper> (not a component you directly control but one that wraps your custom edge components).

@m0nq
Copy link
Author

m0nq commented Jan 10, 2025

Thank you then. I hope the docs get updated soon. I'll see if this also helps resolve the other issues I've outlined above. 🙏🏽

@m0nq m0nq closed this as completed Jan 10, 2025
@moklick
Copy link
Member

moklick commented Jan 10, 2025

I move this issue to the docs, so that we can handle it there.

@moklick moklick transferred this issue from xyflow/xyflow Jan 10, 2025
@moklick moklick reopened this Jan 10, 2025
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

No branches or pull requests

3 participants