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.

Sign up for GitHub

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

Jump to bottom

(fix) - restore debounceRendering after act #1683

Merged
merged 7 commits into from
Jun 8, 2019
Merged

(fix) - restore debounceRendering after act #1683

merged 7 commits into from
Jun 8, 2019

Conversation

JoviDeCroock
Copy link
Member

Fixes: #1681

@coveralls
Copy link

coveralls commented Jun 5, 2019 •
edited
Loading

Coverage Status

Coverage increased (+0.0005%) to 99.775% when pulling 2bf392a on fix/restoreDebounceRendering into 699111d on master.

marvinhagemeister
marvinhagemeister approved these changes Jun 6, 2019
View reviewed changes
Copy link
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super awesome 👍🎉 That's top notch code golf right here💯

robertknight
robertknight reviewed Jun 6, 2019
View reviewed changes
Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me. I'm just fighting an issue following a Node upgrade and then I'll test with the app/product tests where I ran into the original issue.

robertknight
robertknight requested changes Jun 6, 2019
View reviewed changes
Copy link
Member

@robertknight robertknight left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upon further testing it looks like this partly fixes the problem, but I'm afraid I found a second bug. If options.debounceRendering was not set prior to setupRerender being called, then options. __test__previousDebounce is set to undefined and the cleanup code in teardown does not reset options.debounceRendering.

A possible solution would be to test for the presence of the property in teardown rather than it not being undefined.

@JoviDeCroock
Copy link
Member Author

@robertknight it should be fixed now

robertknight
robertknight approved these changes Jun 6, 2019
View reviewed changes
Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works. LGTM!

@JoviDeCroock JoviDeCroock merged commit f90baf9 into master Jun 8, 2019
@JoviDeCroock JoviDeCroock deleted the fix/restoreDebounceRendering branch June 8, 2019 08:23
@snyk-bot snyk-bot mentioned this pull request Oct 26, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marvinhagemeister marvinhagemeister marvinhagemeister approved these changes

@robertknight robertknight robertknight approved these changes

@andrewiggins andrewiggins Awaiting requested review from andrewiggins

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test utils act() disables normal deferred rendering
6 participants
@JoviDeCroock @coveralls @robertknight @andrewiggins @marvinhagemeister @ForsakenHarmony