Search code examples
javascriptarraysmithril.js

Why is creating a new array instead of emptying the old one causing a problem here?


I'm working on a project in Javascript and hit a problem that I don't understand.
Basically I've written a map generator that is generating maps from noise, that bit is fine and working.
I want the end user to be able to edit this map by placing what I call actions (basically shapes which effect the underlying noise generation) and the system for editing these is causing problems.

I'm using Mithril JS to update a UI to edit properties of the actions and it's mostly working. Part of what I need to be able to do is to click on an action and select it, this will show it in the UI to allow you to edit it. Selecting an action works, as does deselecting or selecting another action, but if I then delete an action it no longer shows in the UI. Here's the relevant snippet of code:

var editModeElements = {
    actions: editedActions.actions,
    selectedAction: [],
    selectedPoint: null,
    update: function() {
        this.actions = editedActions;
        this.clearSelection();
        m.redraw();
    },
    deleteAction: function(action) {
        for( var i = 0; i < this.actions.actions.length; i++)
        { 
            if ( this.actions.actions[i] == action) 
            { 
                this.actions.actions.splice(i, 1); i--; 
            }
        }
        this.clearSelection();
        unsavedEdits = true;
    },
    selectAction: function(action) {
        this.selectedAction = new Array();
        this.selectedPoint = null;
        this.selectedAction.push(action);
        m.redraw();
        viewport.redraw();
    },
    clearSelection: function() {
        this.selectedAction = new Array();
        this.selectedPoint = null;
        m.redraw();
        viewport.redraw();
    },
    addAction: function(actionType) {
        var action = createBlankAction(actionType);
        this.actions.actions.push(action);
        this.selectAction(action);
        unsavedEdits = true;
    },
    oncreate: this.update,
    view: function() {
        return m(".action-dialog", [
                m(".action-editor-buttons", [
                    m(".new-action", {onclick: function() {editModeElements.addAction(MASK_TYPE.CIRCLE)}}, "Circle"),
                    m(".new-action", {onclick: function() {editModeElements.addAction(MASK_TYPE.RECTANGLE)}}, "Rectangle"),
                    m(".new-action", {onclick: function() {editModeElements.addAction(MASK_TYPE.POLYGON)}}, "Polygon"),
                    m(".new-action", {onclick: function() {editModeElements.addAction(MASK_TYPE.LINE)}}, "Line"),
                    m(".new-action", {onclick: function() {editModeElements.addAction(MASK_TYPE.COMPLEX_CIRCLE)}}, "Complex Circle"),
                    m(".new-action", {onclick: function() {editModeElements.addAction(MASK_TYPE.COMPLEX_RECTANGLE)}}, "Complex Rectangle"),
                    m(".new-action", {onclick: function() {editModeElements.addAction(MASK_TYPE.COMPLEX_POLYGON)}}, "Complex Polygon"),
                    m(".new-action", {onclick: function() {editModeElements.addAction(MASK_TYPE.COMPLEX_LINE)}}, "Complex Line"),
                ]),
                createActionEditorList(this.selectedAction, this),
        ]);
    }
}

Hopefully that's enough to tell me where I'm going wrong. The issue is something to do with creating a new array when I call clearSelection. If I remove that call from deleteSelection the problem goes away (but the deleted element remains selected in the UI, which is undesirable) or if I change the part in clearSelection to pop the element instead of creating a new array it works.

However, and this is what confuses me, clearSelection works fine (new array or pop method) when I use it to deselect by clicking outside an action and I also create a new Array in selectAction which causes no issues. It only stops working once deleteSelection is called.
The array that the element is deleted from in deleteSelection isn't particularly connected to the selectedAction array other than the action which is selected comes from it so I can't see it would cause these issues. I also changed it from filter (which would create a new array) to splice (which doesn't) to see if that helped, but no change.
The actions still select after a delete and change colour, they can also be dragged around successfully and saved so the issue is obviously the connection to the UI.

I don't think anything else outside this snippet is relevant as it's largely self contained, the function createActionEditorList at the end just turns the actions into HTML elements for the UI, works and is quite long so I haven't included it.

Why does creating a new Array break my code?


Solution

  • Mithril.js now recommends that you use closures for stateful components. I would recommend migrating to that style because it is a lot easier to reason about where the state is.

    Mithril documentation outlining closure components:

    In the above examples, each component is defined as a POJO (Plain Old JavaScript Object), which is used by Mithril internally as the prototype for that component's instances. It's possible to use component state with a POJO (as we'll discuss below), but it's not the cleanest or simplest approach.

    Regarding the bug you are having, after some testing I think it has to do with the fact that the this you are using in your methods is not the same as editModeElements. As I understand it, mithril uses the POJO as the protoype for the this used in methods. I think maybe when cutting your code down some parts have been altered that make the error occur slightly different for me when I try to make it run. The crux of the issue though is that this != editModeElements and it makes the code really confusing. Sometimes you might be modifying the prototype and other times you might be modifying the instance.

    Some of the issues:

    1. When you set oncreate: this.update, the this is not editModeElements but the this of the current scope. So most likely this.update is undefined and oncreate is set to undefined and never called.
    2. The this passed to createActionEditorList is not the same object as editModeElements.addAction used in editModeElements.addAction.
    3. You have some methods refer to actions.actions but editModeElements is initialized such that actions: editedActions.actions.

    It is hard to tell what else is going on without the rest of your code. I tried to use POJOs for state at first because I thought that was necessary but closures are way easier to understand. Maybe you could elaborate if you need that style for some reason because it is very difficult to implement reliably.

    Here is what I think you want your component to do rewritten using the closure style, I had to stub a few things to make it work, :

    // STUB
    var MASK_TYPE = {
      CIRCLE: "CIRCLE",
      RECTANGLE: "RECTANGLE",
      POLYGON: "POLYGON",
      LINE: "LINE",
      COMPLEX_CIRCLE: "COMPLEX_CIRCLE",
      COMPLEX_RECTANGLE: "COMPLEX_RECTANGLE",
      COMPLEX_POLYGON: "COMPLEX_POLYGON",
      COMPLEX_LINE: "COMPLEX_LINE",
    };
    
    function createBlankAction(actionMask) {
      // STUB
      return {mask: actionMask};
    }
    
    var EditModeElements = function () {
      var actions = [],
          selectedAction = [],
          selectedPoint = null,
          unsavedEdits = false;
      function deleteAction(action) {
          for( var i = 0; i < actions.length; i++) { 
              if (actions[i] == action)  { 
                  actions.splice(i, 1); i--; 
              }
          }
          clearSelection();
          unsavedEdits = true;
      }
      function selectAction(action) {
          selectedAction.length = 0;
          selectedPoint = null;
          selectedAction.push(action);
          //?viewport.redraw();
      }
      function clearSelection() {
          selectedAction.length = 0;
          selectedPoint = null;
          //?viewport.redraw();
      }
      function addAction(actionType) {
        var action = createBlankAction(actionType);
        actions.push(action);
        selectAction(action);
        unsavedEdits = true;
      }      
      return {
        view: function() {
            return m(".action-dialog", [
                    m(".action-editor-buttons", [
                        m(".new-action", {onclick: function() {addAction(MASK_TYPE.CIRCLE)}}, "Circle"),
                        m(".new-action", {onclick: function() {addAction(MASK_TYPE.RECTANGLE)}}, "Rectangle"),
                        m(".new-action", {onclick: function() {addAction(MASK_TYPE.POLYGON)}}, "Polygon"),
                        m(".new-action", {onclick: function() {addAction(MASK_TYPE.LINE)}}, "Line"),
                        m(".new-action", {onclick: function() {addAction(MASK_TYPE.COMPLEX_CIRCLE)}}, "Complex Circle"),
                        m(".new-action", {onclick: function() {addAction(MASK_TYPE.COMPLEX_RECTANGLE)}}, "Complex Rectangle"),
                        m(".new-action", {onclick: function() {addAction(MASK_TYPE.COMPLEX_POLYGON)}}, "Complex Polygon"),
                        m(".new-action", {onclick: function() {addAction(MASK_TYPE.COMPLEX_LINE)}}, "Complex Line"),
                    ]),
                    m(ActionEditorList, {actions: actions, onDeleteAction: deleteAction}),
            ]);
        }
      };
    }
    
    function ActionEditorList() {
      return {
        view: function (vnode) {
          return m('', vnode.attrs.actions.map(function (a) {
            return m('b', {
              onclick: function () { 
                vnode.attrs.onDeleteAction(a);
              }
            }, a.mask);
          }));
        }
      }
    }
    
    m.mount(document.body, EditModeElements);