Modify diagram in InitialLayoutCompleted shows warnings about transactions

In our application the user can create a diagram. Based on the location and attributes of the nodes, we additionally create a “line chart” below the diagram:

Currently we don’t persist any data for the line chart in the model, it is calculated on demand based on the diagram model data.

The line chart is created in the “InitialLayoutCompleted” event handler by manually adding a part with panels/shaped/textBoxes to the diagram (diagram.add()).
The line chart is updated in the “ModelChanged” event handler by manually recreating the whole contents of the part.

Now we have the issue that warnings are thrown when the user modifies the diagram:

Change not within a transaction: !dEnumValue.Insert elements: Part#7940  new: Panel([object Object])#8847 2
go-debug.js:14 Change not within a transaction: !d location: Part#7940  old: Point(442,695)  new: Point(385,695)
go-debug.js:14 Change not within a transaction: !d position: Part#7940  old: Point(442,695)  new: Point(385,695)
go-debug.js:14 Change not within a transaction: !d desiredSize: Part#7940  old: Size(2207,NaN)  new: Size(2264,NaN)

I don’t understand this, as the InitialLayoutCompleted event should be within a transaction?
Anyways, if I try to wrap my code manually in a transaction, the gojs warnings are gone when changing the diagram. However now the undo/redo is somehow broken and the “Change not within a transaction” warnings are displayed at the time of an undo/redo.

So maybe it is a bad idea to modify the diagram in the InitialLayoutCompleted event handler? What could I do instead?

Your design is just fine. Since you don’t care about undo/redo of the details of that line chart Part, I suggest that you make sure those changes that you make are not recorded by the UndoManager.

One way of doing that is by temporarily setting Diagram.skipsUndoManager to true.

But an easier way of doing that is by setting the line chart’s Part.layerName to “Grid”, because all changes to any object in a temporary Layer are ignored by the UndoManager.

Hi,
I tried the following:

  1. Setting the layerName to “Grid” / setting the layer with a different name to isTemporary = true:
    The warnings are gone, but now the line chart is cut off at the bottom, probably because it is excluded from bounds calculations?
  2. Setting skipsUndoManager to true in the ModelChanged event handler before recreating the line chart:
    No warnings when e.g. moving an item, but a warning when I undo the move:
    Change not within a transaction: !d location: Part#8004 old: Point(285,695) new: Point(35,695)
    The Undo Manager contained one “Layout” event in the history after moving the node.
  3. Starting a new transaction with skipsUndoManager set to true in the ModelChanged event handler before recreating the line chart:
    No warnings when e.g. moving an item, but a warning when I undo the move:
    Change not within a transaction: !d location: Part#8005 old: Point(285,695) new: Point(85,695)
    The Undo Manager contained two “Move” events in the history after moving the node.

It would be the easiest if I could get the first solution to work. Any suggestions?
With the other solutions I don’t know what to do now. Any ideas what could be the issue or what I could further investigate into?

Yes, it’s easy to exclude a Part from the document bounds by setting Part.isInDocumentBounds to false, but it’s hard to include a Part in the document bounds when its in a layer that is Layer.isTemporary. There actually is an internal property on Layer for that, but we haven’t exported or documented it. Maybe we should document it.

For now, I suggest that you just add your chart Part to a regular Layer, either a predefined one if you aren’t using it or a new Layer. Something like:

    const LineChart = $(go.Part,
      { layerName: "Background", position: new go.Point(0, 500), pickable: false, selectable: false },
      . . .
    );
    myDiagram.add(LineChart);

And make any change to that Part while skipsUndoManager is true:

myDiagram.commit(diag => {
  ... modify LineChart ...
}, null);  // the null means skipsUndoManager is true during this whole transaction

Okay, now in the modelChanged event handler I do all the changes to my manually managed line chart in a new transaction with skipUndoManager=true.
One change I do there is updating the part.position, which seems to cause issues when using undo/redo:

  1. User moves a “normal” node of the diagram
  2. The modelChanged event is triggered (for update of the node location)
  3. From within the modelChanged event, the part.position of the line chart is updated (among others) in a new transaction with skipsUndoManager = true
  4. Now the user performs undo (I can see there is exactly one Move undo element in the undo history)
  5. The “normal” node moves back to it’s initial position, but also a warning is displayed "Change not within a transaction: !d location:..." and the undo history now contains a Layout entry (which does nothing if executed).

All other changes I do within the line chart part (e.g. removing and readding panels) work just fine, it only seems to be the update of the position which causes the warning/and additional undo history entry.

What could cause this behaviour?

I don’t know – have you set a break point there to see what’s going on then?

Okay so in order to redraw the line chart when a undo of a node move on the main diagram is performed, I am doing this:

const itemPositionUndo =
            e.change === go.ChangedEvent.Transaction &&
            (e.propertyName === "FinishedUndo" || e.propertyName === "FinishedRedo") &&
            e.object?.name === "Move";
if (itemPositionUndo) {
	this.lineChart.recreate(diagram);
}

The lineChart.recreate() creates a new transaction with skipUndoManager=true.
However if I do this, I only see modelChanged events for StartingUndo and then FinishedUndo. Then I get a Change not within a transaction warning.

Now if I wrap the lineChart.recreate() into a setTimeout(), I get Started/Committing/CommittedTransaction events after the Undo events, and no warning. No additional Layout entry is created in the history. Everything works as desired.

Why is this behaviour occuring and can I solve the issue without using setTimeout()?

When a move happens, the DraggingTool raises a “SelectionMoved” (or “SelectionCopied”, if allowed) DiagramEvent. At that time you can update your lineChart.

Separately, when a ChangedEvent happens indicating an undo or redo has finished, you can update your lineChart. e.change === go.ChangedEvent.Transaction && (e.propertyName === "FinishedUndo" || e.propertyName === "FinishedRedo").

A simpler, more comprehensive, but less efficient, solution would be to only implement:

$(go.Diagram, . . .,
    { . . .,
      "ModelChanged": e => e.isTransactionFinished && lineChart.recreate()
    })

Basically, update the lineChart after each transaction or undo or redo; no checking for the kind of transaction is needed.

Hello @walter,

I was able to reproduce the issue on codepen. It seems to be caused by a combination of recreating the part contents and changing the parts position at the same time.

See here: https://codepen.io/dominic-simplan/pen/abBzadd?editors=1010
Just move one of the standard nodes and then Undo the change. The console will print a warning:
Change not within a transaction: !d location: Part#547 old: Point(70,70) new: Point(60,60)

I’ll try it in my own app later today.

This seems to work as I understand what you want:

<!DOCTYPE html>
<html>
<head>
  <title>Minimal GoJS Sample</title>
  <!-- Copyright 1998-2021 by Northwoods Software Corporation. -->
  <script src="go.js"></script>
  <script id="code">
  function init() {
    var $ = go.GraphObject.make;

    myDiagram =
      $(go.Diagram, "myDiagramDiv",
        {
          padding: new go.Margin(10, 10, 120, 10),  // enough extra height for LineChart underneath documentBounds
          "undoManager.isEnabled": true,
          "ModelChanged": function(e) {
            if (e.change === go.ChangedEvent.Transaction) {
              if (e.propertyName === "CommittingTransaction" ||  // "CommittedTransaction" is too late
                  e.propertyName === "FinishedUndo" || e.propertyName === "FinishedRedo") {
                updateLineChart();
              }
            }
          }
        });

    var LineChart =
      $(go.Part,  // in a layer that is Layer.isTemporary, so it is not pickable or selectable or isInDocumentBounds
        { layerName: "Grid", isLayoutPositioned: false },  // positioned by "ModelChanged" listenere, not by any Layout
        $(go.Shape, { fill: null, stroke: "red" })
      );
    myDiagram.add(LineChart);

    function updateLineChart() {
      if (LineChart) {
        // don't need transaction because it's happening at "CommittingTransaction" time (so still in some transaction)
        // and it's in isTemporary Layer so that all changes to it are skipped by the UndoManager
        var b = myDiagram.documentBounds.copy().subtractMargin(myDiagram.padding);
        LineChart.position = new go.Point(b.x, b.bottom + 10);
      }
    }

    myDiagram.nodeTemplate =
      $(go.Node, "Auto",
        new go.Binding("location", "loc", go.Point.parse).makeTwoWay(go.Point.stringify),
        $(go.Shape,
          { fill: "white" },
          new go.Binding("fill", "color")),
        $(go.TextBlock,
          { margin: 8 },
          new go.Binding("text"))
      );

    myDiagram.model = new go.GraphLinksModel(
    [
      { key: 1, text: "Alpha", color: "lightblue" },
      { key: 2, text: "Beta", color: "orange" },
      { key: 3, text: "Gamma", color: "lightgreen" },
      { key: 4, text: "Delta", color: "pink" }
    ],
    [
      { from: 1, to: 2 },
      { from: 1, to: 3 },
      { from: 2, to: 2 },
      { from: 3, to: 4 },
      { from: 4, to: 1 }
    ]);
  }
  </script>
</head>
<body onload="init()">
  <div id="myDiagramDiv" style="border: solid 1px black; width:100%; height:600px"></div>
  The red square is always kept at the left edge of the document bounds and just underneath it.
  The red square's Part is in the "Grid" Layer, so it is not pickable or selectable,
  nor do any changes to it get recorded by the UndoManager.
  That Part is also not laid out by any Layout, but by the updateLineChart function.
  That Part is not in the model either, so it is not saved and loaded with the model.
</body>
</html>

Thanks for the feedback.
One question before I try this: As discussed above, I couldn’t put the line chart into a temporary layer as it will be cut off:

How does your solution prevent the line chart from being cut off? Is this solved by the isLayoutPositioned?

Setting Part.isLayoutPositioned to false prevents any normal Layout from considering moving it.

I just increased the Diagram.padding so as to always leave enough room at the bottom. In my sample it’s taking 10 + 100 height, so plus the normal 10 padding, that means Margin.bottom should be 120.

Hello walter,
I am not really sure this solution/workaround works for me:

First, we sometimes display multiple line charts, so I’d need to calculate the padding dynamically. This is not nice but I guess it’s doable.
Second, the line chart would not be included in the image export as far as I can see. There is an option to export the temporary / grid layer as well but this would also export other stuff I don’t want.
And possibly there are other side-effects?

Maybe you could also explain what the underlying cause for this behaviour is. Shouldn’t I modify parts in the UndoFinished handler? Shouldn’t I remove elements for a part? Shouldn’t I change the size of a part? Shouldn’t I start a transaction which skipsUndoManager?

Is this a bug in goJS and I can just wait for the next version? Is this a restriction/principle of the current design of goJS?

To know this would also be useful to circumvent such issues in the future.

OK, I’ve put the LineChart Part back in the “Background” Layer. The complication is that the Diagram.documentBounds now includes the LineChart, but its positioning must be computed without taking the LineChart itself into account.

<!DOCTYPE html>
<html>
<head>
  <title>Minimal GoJS Sample</title>
  <!-- Copyright 1998-2021 by Northwoods Software Corporation. -->
  <script src="go.js"></script>
  <script id="code">
  function init() {
    var $ = go.GraphObject.make;

    myDiagram =
      $(go.Diagram, "myDiagramDiv",
        {
          "undoManager.isEnabled": true,
          "ModelChanged": function(e) {
            if (e.change === go.ChangedEvent.Transaction) {
              if (e.propertyName === "CommittingTransaction" ||  // "CommittedTransaction" is too late
                  e.propertyName === "FinishedUndo" || e.propertyName === "FinishedRedo") {
                updateLineChart();
              }
            }
          }
        });

    var LineChart =
      $(go.Part,  // in a layer that is Layer.isTemporary, so it is not pickable or selectable or isInDocumentBounds
        {
          isLayoutPositioned: false,  // positioned by "ModelChanged" listenere, not by any Layout
          layerName: "Background",
          selectable: false, pickable: false  // in document bounds, but user cannot interact with it
        },
        $(go.Shape, { fill: null, stroke: "red" })
      );
    myDiagram.add(LineChart);

    function updateLineChart() {
      if (LineChart) {
        // don't need transaction because it's happening at "CommittingTransaction" time (so still in some transaction)
        myDiagram.skipsUndoManager = true;
        // don't include the LineChart that is in the "Background" Layer, nor anything in the "Foreground" Layer
        var b = myDiagram.computePartsBounds(myDiagram.findLayer("").parts);
        LineChart.position = new go.Point(b.x, b.bottom + 10);
        myDiagram.skipsUndoManager = false;
      }
    }

    myDiagram.nodeTemplate =
      $(go.Node, "Auto",
        new go.Binding("location", "loc", go.Point.parse).makeTwoWay(go.Point.stringify),
        $(go.Shape,
          { fill: "white" },
          new go.Binding("fill", "color")),
        $(go.TextBlock,
          { margin: 8 },
          new go.Binding("text"))
      );

    myDiagram.model = new go.GraphLinksModel(
    [
      { key: 1, text: "Alpha", color: "lightblue" },
      { key: 2, text: "Beta", color: "orange" },
      { key: 3, text: "Gamma", color: "lightgreen" },
      { key: 4, text: "Delta", color: "pink" }
    ],
    [
      { from: 1, to: 2 },
      { from: 1, to: 3 },
      { from: 2, to: 2 },
      { from: 3, to: 4 },
      { from: 4, to: 1 }
    ]);
  }
  </script>
</head>
<body onload="init()">
  <div id="myDiagramDiv" style="border: solid 1px black; width:100%; height:600px"></div>
  The red square is always kept at the left edge of the document bounds and just underneath it.
  The red square's Part is in the "Background" Layer and made not pickable or selectable.
  Any changes to it are not recorded by the UndoManager by setting skipsUndoManager temporarily to true.
  That Part is also not laid out by any Layout, but by the updateLineChart function.
  That Part is not in the model either, so it is not saved and loaded with the model.
</body>
</html>

Hi,

I think you have skipped the part in your example where I “recreate” the line chart. See my comment above:

See here: https://codepen.io/dominic-simplan/pen/WNoQvXb
I have added these two lines to your example in the updateLineChart() method:

removeAllElements(LineChart);
LineChart.add($(go.Shape, "RoundedRectangle", { fill: "lightblue" }));

This displays a warning about a transaction in the console when moving/undoing a move of a node.

Sorry for not reproducing the conditions well enough to reproduce the problem. Performing an undo or a redo is not necessary to cause that warning message to happen.

We’re looking into this. In the meantime, you’ll only get that message when using the debug version of the library, so you can ignore it for now.

Also, you can add this call to ensureBounds:

    function updateLineChart() {
      if (!LineChart) return;
      // don't need transaction because it's happening at "CommittingTransaction" time (so still in some transaction)
      myDiagram.skipsUndoManager = true;
      var pan = LineChart.findObject("PANEL");
      if (pan) { . . . }  // maybe update the contents
      LineChart.ensureBounds();
      LineChart.position = . . .;  // now position it the way that you want
      myDiagram.skipsUndoManager = false;
    }

Thank you, calling ensureBounds solved the issue!

In my case it was a bit more complicated however, as I’m also updating the LineChart height after updating the position, which somehow caused an additional undo to be generated and calling ensureBounds initially didn’t solve the issue.

Didn’t work (two or three undos generated, warnings in console on undo):

LineChart.ensureBounds()
LineChart.position = ...
LineChart.height = ...

However it is working in the following order now:

LineChart.height = ...
LineChart.ensureBounds()
LineChart.position = ...