Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toReact not working as expected #16

Closed
jongacnik opened this issue Nov 25, 2017 · 2 comments
Closed

toReact not working as expected #16

jongacnik opened this issue Nov 25, 2017 · 2 comments

Comments

@jongacnik
Copy link
Member

The current toReact adapter from #15 does not work as expected. It always returns a new component, and therefore always a new nanocomponent.

If you convert the adapter to use es6 classes, it will work as expected:

function toReact (Component, react) {
  assert.equal(typeof Component, 'function', 'nanocomponent-adapters/react: component should be type function')
  assert.equal(typeof react, 'object', 'nanocomponent-adapters/react: react should be type object')

  class Clx extends react.Component {
    constructor () {
      super()
      this.comp = new Component()
      this.node = null
      this.setRef = this.setRef.bind(this)
    }

    componentDidMount () {
      if (!this.comp.element) {
        var el = this.comp.render(this.props)
        this.node.appendChild(el)
      }
    }

    setRef (_node) {
      this.node = _node
    }

    componentWillReceiveProps (props) {
      if (this.comp.element) this.comp.render(props)
    }

    shouldComponentUpdate () {
      return false
    }

    render (props) {
      return react.createElement('div', { ref: this.setRef })
    }
  }

  return Clx
}

Here's a little demo. If you increment both components, and then trigger a full app re-render, the prototype based component will remount and es6 based will remain:

demo: https://preact-adapter-test.glitch.me
code: https://glitch.com/edit/#!/preact-adapter-test?path=public/client.js:1:1

If we're open to using es6 classes I can open a PR request, if not, I was getting a bit stumped by the prototypes so maybe someone can help take a look!

goto-bus-stop added a commit that referenced this issue Jan 5, 2018
Preact uses the constructor property to associate the correct component
with elements.
@goto-bus-stop
Copy link
Member

oh dang i missed this. i don't know if ES classes are okay with everyone but i fixed the mistake that caused this in #17

@jongacnik
Copy link
Member Author

rad! no need to swap to es classes if this fixes it. I was just having trouble pinpointing where the mistake was. lgtm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants