sojinantony01/react-cron-generator

Component does not react to changes in value outside of the Component

MrSauceman opened this issue · 6 comments

If I change the value of the cron from outside the component, the component does not update its value. This is because it reads the value from props into state from within ComponentWillMount, and that function is only called once on the first render.

It should update the state with the new value every time it changes externally.

It could either implement static getDerivedStateFromProps(props, state) to update the state when the props value changes or just use the value directly from the props and skip the state. Looking at the code, you don't really need the value stored in the state. You could add a function to compute the value from the props and call it where you need it instead of storing it in the state.

An example use case here, is that I have a list of schedules with edit buttons, and one CronGenerator component. When I click on a schedule to edit it, I need to update the CronGenerator with this schedule's cron. This does not work because the CronGenerator only checks my value prop in ComponentWillMount.

@MrSauceman Thank you for reporting this, will fix this ASAP

@MrSauceman Just updated the component,

Using getDerivedStateFromProps need some major change in the component, it may cause other issues

i have added ref to the component now you can access state and functions using ref. You can change cron value from outside the component even after cron generator loaded,

add the new prop onRef={ref => (this.cronGen = ref)}

      <Cron
        onRef={ref => (this.cronGen = ref)}  
        onChange={(e)=> {this.setState({value:e}); console.log(e)}}
        value={this.state.value}
        showResultText={true}
        showResultCron={true}
        />

this.cronGen.setValue(/* Your new value */) 

Just call this.cronGen.setValue(/* Your new value */) to change the value when you need

please use v1.2.3

I was actually able to fix this using componentWillReceiveProps.

I cloned your previous version and made the following minor changes to a few lines and now the value updates properly without having to manually call this.cronGen.setValue(/* Your new value */).

I think these changes are more in the spirit of react, where your component re-renders automatically if the props change. Calling a separate function from a ref to update a prop just doesn't feel quite right for react.

If you want, I can PR this, or you can manually make these changes. They are very minor and will not break anything - I simply extracted the code from componentWillMount into its own function called setValue, called it from componentWillMount, and then added a call to it in componentWillReceiveProps.

cron fix

Let me know what you think.

Yes i have already tried this componentWillRecieProps method
but it made some other mistakes like, when the onChange event called from inside the compoent to parent this will also call componentWillRecieveProps and running all those checks again, (at the initial time if no value supplied to the component this force to change to component to show the default tab)
This may create issues in other users

Yes i know creating ref is not suggested in react but as instant solution i am using this
i will definitely try to clear this in a better way as you suggested

Thank you

That if statement within componentWillReceiveProps that checks to make sure that the value prop has actually changed before calling setValue should take care of the problem you are describing. It will only set the value if it has changed, which is what you want. You want to keep your value state up to date with the value prop that is passed in. I think you want those checks to run every time the value prop changes to make sure you have the latest value.

I have updated the code as you mentioned
it looks working now
it was some other mistake which i found before
Thank you @MrSauceman