projectstorm / react-diagrams

a super simple, no-nonsense diagramming library written in react that just works
https://projectstorm.cloud/react-diagrams
MIT License
8.65k stars 1.17k forks source link

Fix CI failure on master and bug `Cannot read property 'reportedPosition' of null on link.setSourcePort and link.setSourceTarget` #838

Closed vadimshvetsov closed 3 years ago

vadimshvetsov commented 3 years ago

Checklist

What?

We've got an error Uncaught TypeError: Cannot read property 'reportedPosition' of null at DefaultLinkModel.setSourcePort when we use link.clearPort(port)

Why?

Because we call then link.setSourcePort(null) or link.setTargetPort(null) to remove port from link. https://github.com/projectstorm/react-diagrams/blob/d2f05438b8620147d7aa835f2799a0f56397d4e3/packages/react-diagrams-core/src/entities/link/LinkModel.ts#L156 https://github.com/projectstorm/react-diagrams/blob/d2f05438b8620147d7aa835f2799a0f56397d4e3/packages/react-diagrams-core/src/entities/link/LinkModel.ts#L158 But this methods doesn't check port existence before using reportPosition property. https://github.com/projectstorm/react-diagrams/blob/d2f05438b8620147d7aa835f2799a0f56397d4e3/packages/react-diagrams-core/src/entities/link/LinkModel.ts#L227 https://github.com/projectstorm/react-diagrams/blob/d2f05438b8620147d7aa835f2799a0f56397d4e3/packages/react-diagrams-core/src/entities/link/LinkModel.ts#L249

How?

At first we need to expect null as input there for type safety. Then we would check port existence before using it's property

Feel good image:

LOL

apatole commented 3 years ago

Is this fix now part of any public release? I am also seeing it and just want to see if upgrading to any new version will fix it? Thanks.

vadimshvetsov commented 3 years ago

@apatole at this moment this is not a part of any public release. We are waiting for @dylanvorster capacity for checking this out.

apatole commented 3 years ago

@vadimshvetsov Thank you for the updates.