Skip to content

Conversation

sophiebits
Copy link
Collaborator

@sophiebits sophiebits commented Nov 4, 2016

Depends on #8204.

Also fixes return value of ReactNativeMount and moves that callback to be after cDM instead of after all updates.

expect(x).toBe(y);
// We expose refs with a bunch of internal props. Yuck. But this should not
// be TopLevelWrapper.
expect(x._currentElement.props.foo).toBe('foo');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we check x.props.foo if this is fiber while we're at it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is RN so I don't think we can right now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It will fail regardless now but once we fix RN this could pass. Otherwise we later have to go through this test. 15 minutes saved later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah but the module is called ReactDOMFeatureFlags. Want me to do it anyway?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually I will just change this to make sure it is the same as a ref on the View.

Copy link
Collaborator

@sebmarkbage sebmarkbage Nov 5, 2016

Choose a reason for hiding this comment

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

Oh I see what you're saying. Just land it as is NBD.

Also fixes return value of ReactNativeMount and moves that callback to be after cDM instead of after all updates.
@sophiebits sophiebits merged commit ef99e7e into facebook:master Nov 5, 2016
@sophiebits sophiebits added this to the 16.0 milestone Nov 5, 2016
expect(UIManager.updateView).toBeCalledWith(3, 'View', { foo: 'bar' });
});

it('should be able to create and update a native component', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this name is duplicated. a bit confusing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

copy pasta, will fix

@gaearon
Copy link
Collaborator

gaearon commented Nov 8, 2016

Is this comment outdated now and can be removed?

acusti pushed a commit to brandcast/react that referenced this pull request Mar 15, 2017
Also fixes return value of ReactNativeMount and moves that callback to be after cDM instead of after all updates.
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.

5 participants