NorthwoodsSoftware / gojs-angular

A set of Angular components to manage GoJS Diagrams, Palettes, and Overviews
Other
61 stars 16 forks source link

Cannot add property __gohashid, object is not extensible #49

Closed BerenOneHand closed 2 years ago

BerenOneHand commented 2 years ago

Once again I am faced with this error. After a lot of tracing I found that it occurs when you bind an itemarray to a property on the nodedata. When a change is triggered on the node data and the updatetargetbindings gets called, it tries to add gohashid to the itemarray object and the objects inside the array for some reason? It is not two way binding.

Since gojs angular uses immutable data, it then breaks.

createPortArea(direction: string, alignment: go.Spot, portCollection: string): go.GraphObject {
        const portTemplate = new PortTemplate(this.canvasService);
        return goMake(go.Panel, direction, {
            name: portCollection,
            alignment,
            itemTemplate: portTemplate.getPortTemplate(alignment)
        } as go.Panel,
        new go.Binding('itemArray', '', (node: NodeData, target: go.GraphObject): PortData[] => node[portCollection])

        );
    }
WalterNorthwoods commented 2 years ago

Yes, GoJS needs to be able to add the __gohashid property to any Object that is added to a go.Set or go.Map. We had to implement our own Set and Map classes because they hadn't even been proposed yet when we started the GoJS project.

BerenOneHand commented 2 years ago

But the issue is that the data is immutable. So these cannot be added in gojs angular. Does this mean that you cannot use itemarray and itemtemplate in gojs-angular?

WalterNorthwoods commented 2 years ago

The GoJS library only operates on mutable data, for performance reasons. Version 2 of the gojs-angular components works with immutable data, by giving GoJS a copy of the data that it can modify. Are you using version 2 of the gojs-angular package?

If so, perhaps the copies are automatically freezing themselves. Do the copied data have to be instances of that class?

BerenOneHand commented 2 years ago

I am using the latest gojs and gojs-angular packages.

The copied data doesnt have to be instances of that class.

WalterNorthwoods commented 2 years ago

So, you could either continue having the copy be an instance of that class, or you could have the copy create a plain JavaScript Object.

Does that class'es constructor freeze the object? Does it need to?

BerenOneHand commented 2 years ago

The constructor doesn't freeze the object. The objects are made and passed into immer that produces the state going into gojs angular.

WalterNorthwoods commented 2 years ago

I'm wondering what's going on. We've created a StackBlitz sample to experiment: https://stackblitz.com/edit/angular-ivy-dcrl4d?file=src%2Fapp%2Fapp.component.ts How is this different from your situation?

BerenOneHand commented 2 years ago

Just for testing sake, I have forked and edited the link you provided. The onclick event contains code similar to what causes the issue on my end. However, I add a node in the linking tool ( when you create a link going to an empty space, it creates a default node for you ). I added the node data and the port data so you can see how those classes look.

The issue that I have with your sample, is that the click event doesnt even update the diagram ( on either of the two methods of adding a node )

test

WalterNorthwoods commented 2 years ago

I've waited a couple minutes for that stackblitz.io page to load, and I'm still waiting.

Does that exception happen on that new node data that you have added? How do you add that data object?

BerenOneHand commented 2 years ago

Sorry, here is the link to the editor part: link

The exception happens on an already present node on the diagram. I do not know what internals causes the immutable data object to be used inside the itemarray. It is a very simple case on my side. In the insertLink part of the linking tool, I add a new node to the diagram using diagram.model.addNodeData(nodeData);. I also change the portdata of one of the items in the item array (which is possibly the cause). After this, I add a new node by dragging something onto the canvas, which throws the error I have shown.

BerenOneHand commented 2 years ago

I am not certain what you mean by how i add the data object?

rjohnson465 commented 2 years ago

Hi Beren,

We discovered a bug in gojs-angular that would create duplicate link/node additions last Friday. I released version 2.0.6 which should address this bug, as well as improve performance a bit. Can you try it and let me know if your issue persists?

BerenOneHand commented 2 years ago

It seems as though the bug you fixed also fixed this issue.

Thank you

austalakov commented 1 year ago

I find this error still always happens when binding to itemArray and I have to do this every time, where not only the array itself gets reassigned using the spread operator, but also all the items within it (which is probably bad for performance but we don't have big data sets): new go.Binding("itemArray", "items", (items) => [...items].map(item => item = {...item}))