Skip to content

Conversation

vjeux
Copy link
Contributor

@vjeux vjeux commented Sep 7, 2016

In order to properly type an Operation, we need to change the call site from having two arguments: one for type and one for payload into an object that contains both. This isn't a perf regression because we were already constructing this object in the first place and doesn't change the emitted event so shouldn't affect the dev tools.

None of the call sites are actually flow-ified so it isn't technically used but once we will, it'll make sure that we don't send random strings and payload through those very generic methods.

@vjeux
Copy link
Contributor Author

vjeux commented Sep 7, 2016

cc @flarnie @spicyj

@vjeux vjeux force-pushed the type_ReactHostOperationHistoryHook branch from 799b370 to 7cbdbc1 Compare September 7, 2016 02:11
{type: 'update styles', payload: any /* Style Object */} |
{type: 'update attribute', payload: {[name: string]: string}} |
{type: 'remove attribute', payload: string}
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this kind of things I really enjoy using flow :)

In order to properly type an `Operation`, we need to change the call site from having two arguments: one for `type` and one for `payload` into an object that contains both. This isn't a perf regression because we were already constructing this object in the first place and doesn't change the emitted event so shouldn't affect the dev tools.

None of the call sites are actually flow-ified so it isn't technically used but once we will, it'll make sure that we don't send random strings and payload through those very generic methods.
@vjeux vjeux force-pushed the type_ReactHostOperationHistoryHook branch from 7cbdbc1 to ffbe93b Compare September 8, 2016 03:07
@vjeux vjeux added this to the 15-next milestone Sep 8, 2016
@vjeux vjeux merged commit eaefd90 into facebook:master Sep 8, 2016
acdlite pushed a commit to acdlite/react that referenced this pull request Sep 9, 2016
In order to properly type an `Operation`, we need to change the call site from having two arguments: one for `type` and one for `payload` into an object that contains both. This isn't a perf regression because we were already constructing this object in the first place and doesn't change the emitted event so shouldn't affect the dev tools.

None of the call sites are actually flow-ified so it isn't technically used but once we will, it'll make sure that we don't send random strings and payload through those very generic methods.
@zpao zpao modified the milestones: 15-next, 15.4.0 Oct 4, 2016
zpao pushed a commit that referenced this pull request Oct 4, 2016
In order to properly type an `Operation`, we need to change the call site from having two arguments: one for `type` and one for `payload` into an object that contains both. This isn't a perf regression because we were already constructing this object in the first place and doesn't change the emitted event so shouldn't affect the dev tools.

None of the call sites are actually flow-ified so it isn't technically used but once we will, it'll make sure that we don't send random strings and payload through those very generic methods.
(cherry picked from commit eaefd90)
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.

3 participants