cultureamp / react-elm-components

Write React components in Elm
https://www.npmjs.com/package/react-elm-components
BSD 3-Clause "New" or "Revised" License
779 stars 54 forks source link

Error when mounting component in two separate routes #4

Closed 5outh closed 7 years ago

5outh commented 7 years ago

Overview

If node is nonexistent in initialize, the following error occurs, and the application breaks:

Uncaught TypeError: Cannot read property 'appendChild' of null

This happens when switching routes with react-router that have the same elm component embedded. It's easy to fix, by just ignoring null nodes:

initialize = (node) => { if (node === null) { return; } /* do stuff */ }

But, I don't know if:

a) That really solves the root of the problem, or b) it is a necessary addition, considering this library is meant as an example for implementors.

Ideas?

klazuka commented 7 years ago

See #3. Basically, this component doesn't currently handle the full React component lifecycle.

ghost commented 7 years ago

Same issue with storybook

5outh commented 7 years ago

To be clear - I'm mostly looking for clarification on what this library is meant to be - If it's meant as a 20 line starting point for more robust, custom implementations, then nothing needs to be done (except a README update). If it's meant as the go-to library for integrating react with elm, there should be some work done on that front.

I get the feeling that the project is meant as the former, but it's not explicitly mentioned anywhere.

ghost commented 7 years ago

Should be fixed by #5

rtfeldman commented 7 years ago

Fixed in 1.0.1 - thanks @MoeSattler!

As to the question of this library's role, I see it primarily as a simple starting point, but avoiding problems in common cases like this still seems reasonable. 🙂

5outh commented 7 years ago

Thank you both!