Hyperapp: Question about onremove

[cdeutmeyer]

You guys are probably getting tired of me asking silly questions. :)
I'm creating an Ace interop example for a presentation and not seeing the onremove fire when removing DOM elements.. I'm thinking I need to somehow properly dispose of Ace. Here's my code.

const AceComponent = ({ model, actions}) => {  

    const css = {
        height: "200px", 
        width: "800px",
        marginTop: "20px",
        marginBottom: "20px"
    }

    const setupEditor = (e) => {
        const editor = ace.edit(e);
        editor.setTheme("ace/theme/cobalt");
        editor.getSession().setMode("ace/mode/javascript");
        editor.setValue(model.editorText, 1);
        editor.getSession().on('change', _ => actions.setEditorText(editor.getSession().getValue()))                               
    }

    const disposeEditor = (e) => {
        console.log("dispose editor?");
        // ace.editor(e).destroy(),
    }

    return (
      <div>
          <div style={css} oncreate={setupEditor} onremove={disposeEditor}></div>
          <div>{ model.editorText }</div>
      </div>
    )
}

app({ 
  model: {
    showEditor: true,
    editorText: "// welcome to the ace editor \rconsole.log(\"hyperapp is the bee's knees!\");"
  },
  reducers: { 
    setEditorText: (model, text) => ({ editorText: text }),
    toggleEditor: (model) => ({showEditor: !model.showEditor})
  },
  view: (model, actions) => (
      <div>
          <input type="button" value="toggle editor" onclick={ _ => actions.toggleEditor() } />
          { model.showEditor && (<AceComponent model={model} actions={actions}></AceComponent>)}          
      </div>
   )  
})

[jorgebucaran]

@cdeutmeyer I think toggling the Editor like that does not remove the element. The current implementation may be wrong though.

See this CodePen and let me know if it makes sense.

[cdeutmeyer]

I've simplified and adjusted my code to try and get it to work. I'm only seeing "update" fire.
When I swap out my code with your's in codepen, yours work's fine. I don't understand it.

app({ 
  model: {
    showEditor: true,
  },
  reducers: { 
    toggleEditor: (model) => ({showEditor: !model.showEditor})
  },
  view: (model, actions) =>         
        <div>    
            <button type="button" onclick={ _ => actions.toggleEditor() }>toggle editor</button>
            { model.showEditor ? 
              <div>test</div> 
              : 
              <div onremove={_ => console.log("remove")} 
                   onupdate={_ => console.log("update")} 
                   oncreate={_ => console.log("create")}
                >Editor</div>
            }          
        </div>     
})

[jorgebucaran]

@cdeutmeyer Here is where removeChild is called and before that, where we call onremove.

We're also using document.replaceElement, but I don't know if we should call onremove for an element that's not going to be deleted. But, if the element will be recycled, then maybe we should? 🤔

[cdeutmeyer]

I think that from the perspective of a HyperApp end user, it seems reasonable that when the element "goes away" that onremove would then be called.

[jorgebucaran]

@cdeutmeyer That sounds reasonable, on the other hand, why do you (think you) need the onremove?

[cdeutmeyer]

@jbucaran The only thing I think I would ever need it for is proper disposal of 3rd party libraries like the example above using Ace. Maybe I'm over thinking it or being overly paranoid because of all the cleanup code we need to write in our good old jQuery and Knockout code at work.

[jorgebucaran]

@cdeutmeyer Sorry, I meant this as a general question, what I am trying to ask is: why would one need to use onremove?

The only thing I think I would ever need it for is proper disposal of 3rd party libraries like the example above using Ace.

Exactly, why is this necessary? JavaScript has GC, or if you are _toggling_ the editor, then you can call your disposeResources() function or similar inside that toggle effect.

Am I making sense?

[cdeutmeyer]

@jbucaran I guess I am a little confused. It would be cool if there were an example of the proper usage of a third party library with the understanding that the component may be conditionally rendered. Think of a tabbed interface for example. I am aware of the gomix CodeMirror example but that one is harder to follow because you have to hunt through the bundle to see the source. I just want to say that despite these little intricacies, I really am loving what HyperApp is and what you guys are doing!

[jorgebucaran]

I just want to say that despite these little intricacies, I really am loving what HyperApp is and what you guys are doing!

What do you mean?

[jorgebucaran]

OK, my bad for not posting a link to the source code, I missed that one!

https://gomix.com/#!/project/hyperapp-code-mirror

[jorgebucaran]

Think of a tabbed interface for example.

Good idea, looks like a useful example to add to the Examples page. Maybe you could make something?

I'm working on a REPL for HyperApp atm.

[jorgebucaran]

Maybe related #156.

Even if this is not critical, onRemove is probably not correct either. We do call onRemove before we call removeChild ourselves, but we don't when using replaceChild, which also removes the child if it's already on the DOM.

Fairly simple fix, the refactoring involved to keep things tidy requires some careful investigation.

[jorgebucaran]

Let's track this on #156. I'm really not sure what the best approach to take is, but I know we'll figure it out eventually.

[jorgebucaran]

@cdeutmeyer Still a bug, but it will be finally fixed in #317! 🙇

[jorgebucaran]

Finally fixed in #317. 💪

[cdeutmeyer]

Awesome!