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

props not merged when shallow rendering in lifecycleExperimental #1088

Conversation

LarsHassler
Copy link
Contributor

@LarsHassler LarsHassler commented Aug 22, 2017

When shallow rendering a component with lifecycleExperimental: true the lifecyle methods componentWillReceiveProps and shouldComponentUpdate will only be called with the value given to setProps, but not with old and new props merged.

It works for shallow rendering without lifecycleExperimental.

Due to the "dirty hacks" i'm not sure if there is a better way to do this, but this at least fixes the dirty hacks.

@LarsHassler LarsHassler reopened this Aug 22, 2017
@LarsHassler LarsHassler changed the title Fix/props not merged when lifecycle experimental props not merged when shallow rendering in lifecycleExperimental Aug 22, 2017
@@ -262,7 +262,7 @@ class ShallowWrapper {
const state = instance.state;
const prevProps = instance.props || this[UNRENDERED].props;
const prevContext = instance.context || this[OPTIONS].context;
const nextProps = props || prevProps;
const nextProps = Object.assign({}, prevProps || {}, props || {});
Copy link
Member

Choose a reason for hiding this comment

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

The fallback objects are not required here, and this should use object spread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Never tried without the fallback, good to know ;)

Updated the syntax

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Thanks! It'd be great to rebase this down to one commit; otherwise I can do it prior to merging.

@LarsHassler LarsHassler force-pushed the fix/props-not-merged-when-lifecycleExperimental branch from a0a1096 to d9e50bb Compare August 23, 2017 06:03
@LarsHassler
Copy link
Contributor Author

Commits are rebased now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants