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

Fix initial render #136

Merged
merged 1 commit into from
Dec 24, 2018
Merged

Fix initial render #136

merged 1 commit into from
Dec 24, 2018

Conversation

thisRaptori
Copy link
Collaborator

When rendering as a side-effect, the first side-effect-render was not visible, but subsequent renders were. Example:

import React from 'react'
import { interval } from 'rxjs'
import { map } from 'rxjs/operators'
import { withEffects, Aperture } from 'refract-rxjs'

const aperture: Aperture<any, any> = (component) =>
    interval(5000).pipe(
        map(val => <div>{val}</div>),
    )

export default withEffects(aperture)(() => <div>initial</div>)

This would render <div>initial</div> first, then 10 seconds later it would render <div>1</div>, then increasing val values every 5 seconds. The expected <div>0</div> was skipped.

Looks like this is because the havePropsChanged function didn't compare state.children when oldState.renderEffect was false, so shouldComponentUpdate was effectively returning false until the second side-effect render.

Fixed this by also checking newState.renderEffect - tested and this works, and I can't see any other issues this would cause!

@thisRaptori thisRaptori added bug Something isn't working react Something related to React packages preact Something related to Preact packages inferno Something related to Inferno packages labels Dec 23, 2018
@troch
Copy link
Collaborator

troch commented Dec 24, 2018

👍

@troch troch merged commit 49ec93b into master Dec 24, 2018
@troch troch deleted the fix/initial-render branch December 24, 2018 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working inferno Something related to Inferno packages preact Something related to Preact packages react Something related to React packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants