-
Notifications
You must be signed in to change notification settings - Fork 47.5k
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
RFC #30: React.forwardRef implementation #12346
Merged
Merged
Changes from 5 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
9bd0fdb
RFC 30: React.useRef
bvaughn 753dba7
Added error handling tests
bvaughn 56138d2
invariant() -> warning()
bvaughn 60efe32
Changed warning wording slightly
bvaughn 166d236
Added invariant with component stack (in dev) for useRef without func…
bvaughn 0d565b5
Renamed files useRef -> forwradRef
bvaughn 508d52e
Find and replace useRef => forwardRef
bvaughn d5a196d
Changed implementation to avoid creating a fiber
bvaughn 3583bf6
USE_REF => FORWARD_REF
bvaughn 7c3d823
Removed recursion from ReactChildFiber for forwardRef
bvaughn 2661583
Merge branch 'master' into use-ref
bvaughn 6185838
Reverted accidental line reordering
bvaughn 6d7d1ef
forwardRef is now a new fiber/type again
bvaughn 5321365
Renamed renderFn to render
bvaughn 557cd79
Removed second invariant
bvaughn 04ec720
Added forwardRef type to server renderer
bvaughn 1b1deda
Slightly beefed up SSR forwardRef test case
bvaughn b91b3a3
Symbol.for react.use_ref => react.forward_ref
bvaughn c038d89
Added test to ensure refs can be forwarded between components
bvaughn 5b5b625
Read render to prop before calling
bvaughn 2a7b3c8
Merged master
bvaughn 33bb141
Updated createRef usage to use .current
bvaughn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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 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 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 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ import { | |
Mode, | ||
ContextProvider, | ||
ContextConsumer, | ||
UseRef, | ||
} from 'shared/ReactTypeOfWork'; | ||
import { | ||
PerformedWork, | ||
|
@@ -166,6 +167,23 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>( | |
return workInProgress.child; | ||
} | ||
|
||
function updateUseRef(current, workInProgress) { | ||
const renderProp = workInProgress.type.renderProp; | ||
invariant( | ||
typeof renderProp === 'function', | ||
'useRef requires a render function but was given %s.%s', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This error message isn't great 😄 Any suggestions? |
||
renderProp === null ? 'null' : typeof renderProp, | ||
ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', | ||
); | ||
const nextChildren = renderProp( | ||
workInProgress.pendingProps, | ||
workInProgress.ref, | ||
); | ||
reconcileChildren(current, workInProgress, nextChildren); | ||
memoizeProps(workInProgress, nextChildren); | ||
return workInProgress.child; | ||
} | ||
|
||
function updateMode(current, workInProgress) { | ||
const nextChildren = workInProgress.pendingProps.children; | ||
if (hasLegacyContextChanged()) { | ||
|
@@ -1102,6 +1120,8 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>( | |
return updateFragment(current, workInProgress); | ||
case Mode: | ||
return updateMode(current, workInProgress); | ||
case UseRef: | ||
return updateUseRef(current, workInProgress); | ||
case ContextProvider: | ||
return updateContextProvider( | ||
current, | ||
|
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 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 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 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,202 @@ | ||
/** | ||
* Copyright (c) 2013-present, Facebook, Inc. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
* | ||
* @emails react-core | ||
*/ | ||
|
||
'use strict'; | ||
|
||
describe('useRef', () => { | ||
let React; | ||
let ReactFeatureFlags; | ||
let ReactNoop; | ||
|
||
function normalizeCodeLocInfo(str) { | ||
return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); | ||
} | ||
|
||
beforeEach(() => { | ||
jest.resetModules(); | ||
ReactFeatureFlags = require('shared/ReactFeatureFlags'); | ||
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; | ||
React = require('react'); | ||
ReactNoop = require('react-noop-renderer'); | ||
}); | ||
|
||
it('should work without a ref to be forwarded', () => { | ||
class Child extends React.Component { | ||
render() { | ||
ReactNoop.yield(this.props.value); | ||
return null; | ||
} | ||
} | ||
|
||
function Wrapper(props) { | ||
return <Child {...props} ref={props.forwardedRef} />; | ||
} | ||
|
||
const RefForwardingComponent = React.useRef((props, ref) => ( | ||
<Wrapper {...props} forwardedRef={ref} /> | ||
)); | ||
|
||
ReactNoop.render(<RefForwardingComponent value={123} />); | ||
expect(ReactNoop.flush()).toEqual([123]); | ||
}); | ||
|
||
it('should forward a ref to a chosen component', () => { | ||
class Child extends React.Component { | ||
render() { | ||
ReactNoop.yield(this.props.value); | ||
return null; | ||
} | ||
} | ||
|
||
function Wrapper(props) { | ||
return <Child {...props} ref={props.forwardedRef} />; | ||
} | ||
|
||
const RefForwardingComponent = React.useRef((props, ref) => ( | ||
<Wrapper {...props} forwardedRef={ref} /> | ||
)); | ||
|
||
const ref = React.createRef(); | ||
|
||
ReactNoop.render(<RefForwardingComponent ref={ref} value={123} />); | ||
expect(ReactNoop.flush()).toEqual([123]); | ||
expect(ref.value instanceof Child).toBe(true); | ||
}); | ||
|
||
it('should maintain ref through updates', () => { | ||
class Child extends React.Component { | ||
render() { | ||
ReactNoop.yield(this.props.value); | ||
return null; | ||
} | ||
} | ||
|
||
function Wrapper(props) { | ||
return <Child {...props} ref={props.forwardedRef} />; | ||
} | ||
|
||
const RefForwardingComponent = React.useRef((props, ref) => ( | ||
<Wrapper {...props} forwardedRef={ref} /> | ||
)); | ||
|
||
let setRefCount = 0; | ||
let ref; | ||
|
||
const setRef = r => { | ||
setRefCount++; | ||
ref = r; | ||
}; | ||
|
||
ReactNoop.render(<RefForwardingComponent ref={setRef} value={123} />); | ||
expect(ReactNoop.flush()).toEqual([123]); | ||
expect(ref instanceof Child).toBe(true); | ||
expect(setRefCount).toBe(1); | ||
ReactNoop.render(<RefForwardingComponent ref={setRef} value={456} />); | ||
expect(ReactNoop.flush()).toEqual([456]); | ||
expect(ref instanceof Child).toBe(true); | ||
expect(setRefCount).toBe(1); | ||
}); | ||
|
||
it('should not break lifecycle error handling', () => { | ||
class ErrorBoundary extends React.Component { | ||
state = {error: null}; | ||
componentDidCatch(error) { | ||
ReactNoop.yield('ErrorBoundary.componentDidCatch'); | ||
this.setState({error}); | ||
} | ||
render() { | ||
if (this.state.error) { | ||
ReactNoop.yield('ErrorBoundary.render: catch'); | ||
return null; | ||
} | ||
ReactNoop.yield('ErrorBoundary.render: try'); | ||
return this.props.children; | ||
} | ||
} | ||
|
||
class BadRender extends React.Component { | ||
render() { | ||
ReactNoop.yield('BadRender throw'); | ||
throw new Error('oops!'); | ||
} | ||
} | ||
|
||
function Wrapper(props) { | ||
ReactNoop.yield('Wrapper'); | ||
return <BadRender {...props} ref={props.forwardedRef} />; | ||
} | ||
|
||
const RefForwardingComponent = React.useRef((props, ref) => ( | ||
<Wrapper {...props} forwardedRef={ref} /> | ||
)); | ||
|
||
const ref = React.createRef(); | ||
|
||
ReactNoop.render( | ||
<ErrorBoundary> | ||
<RefForwardingComponent ref={ref} /> | ||
</ErrorBoundary>, | ||
); | ||
expect(ReactNoop.flush()).toEqual([ | ||
'ErrorBoundary.render: try', | ||
'Wrapper', | ||
'BadRender throw', | ||
'ErrorBoundary.componentDidCatch', | ||
'ErrorBoundary.render: catch', | ||
]); | ||
expect(ref.value).toBe(null); | ||
}); | ||
|
||
it('should support rendering null', () => { | ||
const RefForwardingComponent = React.useRef((props, ref) => null); | ||
|
||
const ref = React.createRef(); | ||
|
||
ReactNoop.render(<RefForwardingComponent ref={ref} />); | ||
ReactNoop.flush(); | ||
expect(ref.value).toBe(null); | ||
}); | ||
|
||
it('should warn if not provided a callback during creation', () => { | ||
expect(() => React.useRef(undefined)).toWarnDev( | ||
'useRef requires a render function but was given undefined.', | ||
); | ||
expect(() => React.useRef(null)).toWarnDev( | ||
'useRef requires a render function but was given null.', | ||
); | ||
expect(() => React.useRef('foo')).toWarnDev( | ||
'useRef requires a render function but was given string.', | ||
); | ||
}); | ||
|
||
it('should error with a callstack if rendered without a function', () => { | ||
let RefForwardingComponent; | ||
expect(() => { | ||
RefForwardingComponent = React.useRef(); | ||
}).toWarnDev('useRef requires a render function but was given undefined.'); | ||
|
||
ReactNoop.render( | ||
<div> | ||
<RefForwardingComponent /> | ||
</div>, | ||
); | ||
|
||
let caughtError; | ||
try { | ||
ReactNoop.flush(); | ||
} catch (error) { | ||
caughtError = error; | ||
} | ||
expect(caughtError).toBeDefined(); | ||
expect(normalizeCodeLocInfo(caughtError.message)).toBe( | ||
'useRef requires a render function but was given undefined.' + | ||
(__DEV__ ? '\n in div (at **)' : ''), | ||
); | ||
}); | ||
}); |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
/** | ||
* Copyright (c) 2014-present, Facebook, Inc. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
import {REACT_USE_REF_TYPE} from 'shared/ReactSymbols'; | ||
|
||
import warning from 'fbjs/lib/warning'; | ||
|
||
export default function useRef<Props, ElementType: React$ElementType>( | ||
renderProp: (props: Props, ref: React$ElementRef<ElementType>) => React$Node, | ||
) { | ||
warning( | ||
typeof renderProp === 'function', | ||
'useRef requires a render function but was given %s.', | ||
renderProp === null ? 'null' : typeof renderProp, | ||
); | ||
|
||
return { | ||
$$typeof: REACT_USE_REF_TYPE, | ||
renderProp, | ||
}; | ||
} |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow why are these things so easy with Fiber
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know! 😁