Sync latest from Facebook #66
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This expects static children as additional arguments to the constructor and flattens any array arguments one level deep. Component(props, child1, child2, arrayOfChildren, child3) -> .props.children = [child1, child2, ...arrayOfChildren, child3] This can avoid an additional heap allocation for the unflat array. It allows you to pass nested arrays and objects like you used to. Those aren't immediately flattened. That makes this a fairly safe change. Passing a dynamic array without key properties will yield a warning (once). Might consider throwing later. Once we change the transpiler to use the new syntax, you'll end up with a single flat array in normal usage. This doesn't actually update the JSX transform.
Some improvements to how style={{x:y}} is handled in React:
* ignores null styles, rather than setting them.
Codez:
var highlighted = false;
<div style={{color: highlighted ? 'red' : null}} />
Before:
<div style="color:;"></div>
After:
<div></div>
Respects that 0 has no units.
flattenChildren was only using key when child.mountInContainerNode exists, which is defined on ReactCompositeComponent, and not ReactNativeComponent. This uses the isValidComponent() fn to see if we should use this key.
mapChilden() is similar to Array.map() and objMap() but handles deep nested structures and follows similar rules to flattenChildren()
This adds ReactProps.func so people don't need to write the slightly-more-cryptic ReactProps.instanceOf(Function). We should have had this all along.
Currently we're mutating _key. Mutation here is fine, but it needs to be idempotent - which it's not. This is causing some issues. Instead I reassign the _key every time it passes through a flattening. This means that it's unique and stable for a single pass through a composite component. When it's repassed another level, it loses it previous identity and is rekeyed by it's new location. For auto-generated keys by index, this actually means it has the same semantics as before flattening. For explicit keys, it has the effect that keys need to be unique at every level. Regardless of how the key got there. Every component needs to ensure that it doesn't combine keys from two different sources that may collide. This is also inline with the old semantics but less intuitive in the new model.
It seems that the use of Object.create (to comply with strict mode) in NormalizedEventListener is not happy in IE8.
It seems like it's possible to render a component that ends up having an owner. Because you can end up rendering inside a render somehow.
var container = ...; // some DOM node React.renderComponent(<div />, container); React.renderComponent(<span />, container); This should replace the rendered <div> with a <span>, effectively reconciling at the root level.
As it turns out, default values are very useful. This implements getDefaultProps(), a hook for components to provide prop values when a prop is not specified by the user.
Bring back the invariant() that disallows setProps() and replaceProps() on owned components.
If a React component's render() fatals, it may contaminate ReactCurrentOwner. This will cause the owner to be set improperly for the next React.renderComponent() invocation (which causes an owner to be set when there shouldn't be one).
We need to make sure that deleteAllListeners() is invoked before we call the superclass's unmountComponent() method or else we will lose this._rootNodeID. I also added an invariant and unit test to make sure we do not break this in the future.
ReactEvent should be reserved for the actual object created when an event fires. The current ReactEvent is more like EventEmitter than anything (e.g. it sets up delegation, provides methods to attach and remove listeners).
Summary: Since grepping for `update` and `updateAll` is pretty hard, I had these these functions call through but complain loudly. This noisy call through has been in prod for over a week and I haven't heard any complains, so let's take it out altogether.
Summary: This makes a few changes to React Core, most notably `ReactEventEmitter` and `ReactEventTopLevelCallback`. - Changed `ReactEventEmitter` to use `EventListener` (instead of `NormalizedEventListener`). - Deleted `NormalizedEventListener` (which was previously broken). - Created `getEventTarget` which is used to get a normalized `target` from a native event. - Changed `ReactEventTopLevelCallback` to use `getEventTarget`. - Renamed `abstractEventType` to `reactEventType` in `AbstractEvent`. - Reanmed `abstractTargetID` to `reactTargetID` in `AbstractEvent`. - Removed `originatingTopLevelEventType` from `AbstractEvent` (unused and violates encapsulation). - Removed `nativeEvent.target === window` check when refreshing authoritative scroll values (unnecessary). This actually fixes React because `NormalizedEventListener` does not currently do what it promises to do (which is normalizing `target` on the native event). The `target` event is read-only on native events. This also revises documentation and adds `@typechecks` to a few modules. NOTE: Most importantly, this sets the stage for replacing `AbstractEvent` with `ReactEvent` and subclasses, piecemeal.
This improved browser support for the `wheel` event.
- Try to use `wheel` event (DOM Level 3 Specification).
- Fallback to `mousewheel` event.
- Fallback to `DOMMouseWheel` (older Firefox).
Also, since `wheel` is the standard event name, let's use that in React.
NOTE: The tricky part was detecting if `wheel` is supported for IE9+ because `onwheel` does not exist.
Test Plan:
Execute the following in the console on a page with React:
var React = require('React');
React.renderComponent(React.DOM.div({
style: {
width: 10000,
height: 10000
},
onWheel: function() {
console.log('wheel');
}
}, null), document.body);
Verified that mousewheel events are logged to the console.
Verified in IE8-10, Firefox, Chrome, and Safari.
Followup with some additional comments for facebook#58
bvaughn
added a commit
to bvaughn/react
that referenced
this pull request
Aug 13, 2019
Stop mouseup propagation while inspecting
taneliang
referenced
this pull request
in MLH-Fellowship/react
Aug 17, 2020
taneliang
referenced
this pull request
in MLH-Fellowship/react
Aug 19, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the latest. As of right now the JSX transform code is unchanged, though I know Sebastian is landing something there soon (there's currently a test that "passes" but outputs warnings due to the
keystuff). We can either merge this in now or wait for that. Either way is fine by me!