Error when unmounting

[rix501]

Right now, the code uses the ref callback in React to get the node in order to embed the Elm component but after unmounting the callback gets called again, this time with null. This is by design https://facebook.github.io/react/docs/more-about-refs.html . The problem is that when it gets called with null it still tries to embed the Elm component throwing an error:

The component should either use some lifecycle method (like componentDidMount) or do a null check.

Thoughts?

[evancz]

Interesting, thanks for reporting this!

In an upcoming release, there will be a way to kill an Elm app so we can turn everything off properly. Fixing this is blocked on that feature.

[linyanzhong]

It's a bug which prevents the ELM component to work in react-router?

[evancz]

Not sure @linyanzhong.

If you are running into this kind of thing, I'd recommend using Elm.MyThing.embed directly and setting up your ports directly, as described in this section of the guide. That is the actual Elm API.

This package is mainly intended to cover simple cases and reveal to people that embedding Elm is a good way to introduce it into your project.

[linyanzhong]

Just reproduced the react-router problem in the example: Source

• Click "Chat" for the first time, it works
• Click "Hello", the console logs Chat.elm:6546 Uncaught TypeError: Cannot read property 'appendChild' of null, which points to parent.appendChild(domNode);
• Click "Chat" again, the ELM component does not show

[linyanzhong]

It seems possible to use react-router in a tricky way:

Use Elm.MyThing.embed and set up ports directly as mentioned by @evancz above, then write an empty React component, in its componentDidMount and componentWillUnmount, set the ELM component's .style.display to block or none.

[arecvlohe]

I am seeing the same error with my component as well.

@linyanzhong How did you end up fixing your issue, do you have a gist? Did you use refs?

[linyanzhong]

@arecvlohe , if you don't use react-router, and don't mind seeing the warning message in the console, maybe you can continue using this component and wait for the next elm release.

Now I'm using the tricky way described in my last comment: embed the elm component manually like interop javascript, and let react-router control its visibility via a dummy "proxy" react component like this one.

[arecvlohe]

@linyanzhong I was able to get it to work as you described, with the error in the console. It works with react-router, but also slows page speed down a bit.

I am currently using this component in a side project to kind of test it out. If this gets to the point where it can be used in a robust React/Redux application, then I would be encouraged to introduce it at work and not look like a crazy person :)

[klazuka]

I just ran into this bug when trying to integrate a standalone Elm app into a larger React app at work (which uses react-router).

@evancz will 0.18 unblock the fix? Based on reading the mailing list, it sounds like the teardown stuff will not land until 0.19. I love the idea of this React component and the "use Elm at work" blog post, but right now it's difficult to actually use it at-work where it's common to have an app that's complex enough to need a router.

In the interest of not getting blocked, I'm going to try @linyanzhong workaround. But if the real fix is a long way off, perhaps a workaround should be mentioned in the README? @rtfeldman did NoRedInk have similar problems? Any tips?

[ghost]

Same hear. Was super excited to finally be able to use it at work, and now it doesn't work. I guess I'll check Elm.MyThing.embed out.

[ghost]

Shouldn't it be somehow possible to get elm garbage collected before it causes issues ?

[klazuka]

@MoeSattler that's what I'm hoping for--and in my case I'm just doing this on an internal admin page, so it's good-enough for me. I don't know enough about the Elm runtime nor JS internals, but it seems likely that there would be situations where the Elm runtime registers a callback with the Browser, and that without an explicit cleanup/teardown step, it would stay alive and not be able to be collected.

[ghost]

@klazuka
Adding a null check fixes the situation for me. It feels very hacky though:

    initialize: function(node) {
        if (!node) return;
        var app = this.props.src.embed(node, this.props.flags);

        if (typeof this.props.ports !== 'undefined') {
            this.props.ports(app.ports);
        }
    },

Though I don't know, as you pointed out, if the Elm app will be shut down, or you basicly end up with a memory leak of running elm instances.

[kitofr]

Any input on this? Will this cause a memory leak?

[ghost]

If anyone is interested, I am trying to work on an alternative solution:
https://github.com/MoeSattler/reactify-elm

PRs are welcome

[ghost]

@kitofr This proposition was merged with #5 and I never had problems with it in reactify-elm, so I am pretty sure it should work fine.

[kitofr]

Tnx

[choonkeat]

almost there.

I could have a port to handle unmount (stop subscription, minimize memory usage)

subscriptions : Model -> Sub Msg
subscriptions model =
    if model.mounted then
        Sub.batch
            [ unmount Unmount
            , Browser.Events.onResize WindowResize -- we'll stop seeing these after `unmount`
            ]

    else
        -- Elm runtime should unmount listeners and not leak memory
        Sub.none

I could extend the provided Elm component with componentWillUnmount handler...

class UnmountableElm extends Elm {
  componentWillUnmount () {
    /* this.app ? */.ports.unmount.send(null)
  }
}

but I can't invoke that port and tell my app instance to clean up if there's no reference to the app which was created as a local var in initialize

react-elm-components/index.js

Lines 8 to 12 in 879c8ef

	var app;
	if (this.props.src.embed) {
	// Elm 0.18
	app = this.props.src.embed(node, this.props.flags);

PS: yes even after cleanup, my Elm app is still in memory until Elm allows me to remove the instance proper

[choonkeat]

can't invoke that port and tell my app instance to clean up

can be achieved after all

import { unmountWithPort, UnmountableElm } from './UnmountableElm.js'
import Counter from './elm/Counter.js'

function App () {
  return <UnmountableElm
          src={Counter.Elm.Counter}
          ports={unmountWithPort('unmount')}
          />
}

with a wrapper, e.g. UnmountableElm.js

import React from 'react'
import Elm from 'react-elm-components'

var unmountFunctionRefs = {}
export function unmountWithPort (portName, originalHandler) {
  return function (ports) {
    if (originalHandler) originalHandler(ports)
    if (ports && ports[portName]) {
      unmountFunctionRefs[this] = ports[portName].send // `this` refers to the react component instance
    }
  }
}

export class UnmountableElm extends Elm {
  componentWillUnmount () {
    let unmountThis = unmountFunctionRefs[this]
    delete unmountFunctionRefs[this]
    if (unmountThis) unmountThis()
  }
}