Skip to content

Fix useImperativeHandle to have no deps by default #14801

New issue

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

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

Already on GitHub? # to your account

Merged
merged 3 commits into from
Feb 11, 2019

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Feb 8, 2019

Fixes #14782. If nothing is provided, there should be no dependencies — not [ref] alone.

Copy link
Collaborator

@acdlite acdlite left a comment

Choose a reason for hiding this comment

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

You checked the shallow renderer and server renderer too?

@@ -905,7 +905,7 @@ function mountImperativeHandle<T>(

// TODO: If deps are provided, should we skip comparing the ref itself?
const effectDeps =
deps !== null && deps !== undefined ? deps.concat([ref]) : [ref];
deps !== null && deps !== undefined ? deps.concat([ref]) : deps;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
deps !== null && deps !== undefined ? deps.concat([ref]) : deps;
deps !== null && deps !== undefined ? deps.concat([ref]) : null;

Nit: Should only be null, not undefined. I wondered why this wasn't a type error but it's because mountEffectImpl and updateEffectImpl contain their own type check; we might inline those later.

@@ -931,7 +931,7 @@ function updateImperativeHandle<T>(

// TODO: If deps are provided, should we skip comparing the ref itself?
const effectDeps =
deps !== null && deps !== undefined ? deps.concat([ref]) : [ref];
deps !== null && deps !== undefined ? deps.concat([ref]) : deps;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

@gaearon
Copy link
Collaborator Author

gaearon commented Feb 11, 2019

You checked the shallow renderer and server renderer too?

Just checked — both are noops.

@gaearon gaearon merged commit f24a0da into facebook:master Feb 11, 2019
@gaearon gaearon deleted the fix-uih branch February 14, 2019 19:25
This was referenced Sep 20, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants