Olical / react-faux-dom

DOM like structure that renders to React (unmaintained, archived)
http://oli.me.uk/2015/09/09/d3-within-react-the-right-way/
The Unlicense
1.21k stars 87 forks source link

Example not working #97

Closed peterkogo closed 7 years ago

peterkogo commented 7 years ago

The example code does not seem to work for me.

import React from 'react'
import { withFauxDOM } from 'react-faux-dom'
import * as d3 from 'd3'

import style from './NeuroVis.css'

class NeuroVis extends React.Component {

  constructor(props) {
    super(props)
    this.state = {
      chart: 'loading...',
    }
  }

  componentDidMount() {
    const faux = this.connectFauxDOM('div', 'chart')
  }

  render() {
    return (
      <div className={style.content}>
        {this.state.chart}
      </div>
    )
  }
}

export default withFauxDOM(NeuroVis)
TypeError: Cannot read property 'chart' of undefined
    at WithFauxDOM.connectFauxDOM (withFauxDOM.js:30)
    at ProxyComponent.componentDidMount (NeuroVis.js:17)
    at ProxyComponent.proxiedComponentDidMount (createPrototypeProxy.js:66)
    at ReactCompositeComponent.js:265
    at measureLifeCyclePerf (ReactCompositeComponent.js:75)
    at ReactCompositeComponent.js:264
    at CallbackQueue.notifyAll (CallbackQueue.js:76)
    at ReactReconcileTransaction.close (ReactReconcileTransaction.js:80)
    at ReactReconcileTransaction.closeAll (Transaction.js:206)
    at ReactReconcileTransaction.perform (Transaction.js:153)

It seems to be an issue with the line

if (!this.connectedFauxDOM[name] || discardNode) {

However connectedFauxDOM is an existing object.

Am I missing something obvious here, or is this an issue with your library?

tibotiber commented 7 years ago

@PeterKey I create a minimal example from what was shared in the README. I tested it and published it at https://github.com/tibotiber/rfd-min-example. Let me know if that helps 😉.

peterkogo commented 7 years ago

@tibotiber Thank you so much! Indeed it works like charm - which makes my issue a bit more peculiar, having the same component code in my project.

Is the service worker required for react-faux-dom to work?

I am currently using react-static-boilerplate as my starting point and I am just trying to add a component that uses react-faux-dom. Off the top of your head, could anything interfere here?

What really puzzles me is that the error states, that connectedFauxDOM does not exist. The object is created correctly in componentWillMount() but is no longer available when you try to access it during componentDidMount().

Anyway, if there is no easy answer to this, I shall investigate further - clearly this is no issue of the library but something specific to my architecture.

tibotiber commented 7 years ago

@PeterKey the service worker is not needed here, it's a caching optimisation for production that ships by default with create-react-app. May I ask which version of react-faux-dom you are on? Anything higher than 4.0.0 is currently bugged and we're working on it.

fjcalzado commented 7 years ago

Same issue here:

Cannot read property 'chart' of null

I will try downgrading to 4.0.0.

fjcalzado commented 7 years ago

No luck, state keeps being undefined.

tibotiber commented 7 years ago

@PeterKey @fjcalzado if problem persists, would one of you post a minimal example where we can debug this? You can fork https://github.com/tibotiber/rfd-min-example and edit App.js if you find it convenient.

fjcalzado commented 7 years ago

I found the issue @tibotiber , it was my fault, at least in my case. I was trying to create one faux DOM node in the constructor, as a class member, just before componentWillMount() had run which is where the connectedFauxDOM member is initialized as an empty object.

fjcalzado commented 7 years ago

Is there any reason why this.connectedFauxDOM = {} cannot be placed in the constructor instead? This way we could create our faux nodes as class members and have a handy reference to them.

tibotiber commented 7 years ago

@fjcalzado this sounds like a good idea to me. @Olical any opinion?

Olical commented 7 years ago

Sounds good to me too, I have no preference. May as well make that change part of #102.

I'm wondering if this should be v5.0.0, or if it's okay to keep using v4 since I completely broke it and it hasn't worked since the major version bump anyway. Does an API change count if it didn't work? :smile:

tibotiber commented 7 years ago

I think this is not relevant with the new implementation. There used to be a sort of dead zone because the wrapped component and the HOC were the same instance, hence implementation internals of the HOC had an impact on when instance properties were initiated. Now the HOC will instantiate the wrapped component only when its props are ready and we shouldn't have such issues anymore. Am I right here? If yes, we already see the benefits of not using an inheritance inversion 😄.

About the version number, I'd consider we botched 4.0.0 and we're just making a fix so now at 4.0.3. Of course this means the props-based API is supposed to be applicable from 4.0.0. Makes sense? But i'm not religious about these things, go with what feels good to you 😉.

peterkogo commented 7 years ago

I am on version 4.0.0. I have created a fork repository of the boilerplate code I am working with and added a component called FauxElem with your sample code which is now the only component populating the homepage. The same issue arises.

Olical commented 7 years ago

@tibotiber I think you're right, it will recursively mount, so it should be instantiated by the time the child component is mounted. 4.0.3 is fine by me. I'll just pretend that v4.0.{0,1,2} didn't happen :neutral_face:

Olical commented 7 years ago

@PeterKey maybe give this work in progress a go? https://github.com/Olical/react-faux-dom/pull/102

It passes the methods you need through the props and uses true HOC through composition, it should do the trick but you'll have to adjust your client code slightly to use props.

peterkogo commented 7 years ago

@Olical Sure! I'll check it out.

Olical commented 7 years ago

Okay, how's v4.0.3 looking for you, @PeterKey?

peterkogo commented 7 years ago

@Olical @tibotiber Works like charm! Thank you so much!

Olical commented 7 years ago

Wooo! :D

On 3 Jun 2017 11:53, "PeterKey" notifications@github.com wrote:

Closed #97 https://github.com/Olical/react-faux-dom/issues/97.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Olical/react-faux-dom/issues/97#event-1108808121, or mute the thread https://github.com/notifications/unsubscribe-auth/AATPXdZOgAauWTRIX6Vqi50EjgphybEIks5sATsvgaJpZM4NsLMq .