From 2647d5da3781eacb57579001fdea3091d5f5f76d Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Thu, 25 Jan 2018 16:26:44 +0300 Subject: [PATCH 01/11] RFC: New createRef() API for ref objects --- text/0000-new-create-ref.md | 104 ++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 text/0000-new-create-ref.md diff --git a/text/0000-new-create-ref.md b/text/0000-new-create-ref.md new file mode 100644 index 00000000..58b47be6 --- /dev/null +++ b/text/0000-new-create-ref.md @@ -0,0 +1,104 @@ +- Start Date: (fill me in with today's date, 2018-01-25) +- RFC PR: (leave this empty) +- React Issue: (leave this empty) + +# Summary + +Replace string refs with similar functionality to how they currently work but without many of the issues that string refs currently have (a list of them can be found at [#1373](/~https://github.com/facebook/react/issues/1373)). + +# Basic example + +The React.createRef() API will create an immutable object ref (where it's value is a mutable object referencing the actual ref). Accessing the ref value can be done via ref.value. An example of how this works is below: + +```js +class MyComponent extends React.Component { + divRef = React.createRef(); + + render() { + return
; + } + + componentDidMount() { + this.divRef.value.focus(); + } +} +``` + +# Motivation + +This alternative API shouldn't provide any big real wins over callback refs - other than being a nice convenience feature. There might be some small wins in performance - as a common pattern is to assign a ref value in a closure created in the render phase of a component - this avoids that (even more so when a ref property is assigned to a non-existent component instance property). + +One benefit would be more correct Flow types. People tend to type refs incorrectly because Flow doesn't enforce uninitialized class properties correctly: + +```js +class Foo { neverSet: boolean; } +let foo = new Foo(); +(foo.neverSet: boolean); // works +``` + +# Detailed design + +Introduces `React.createRef()`: + +```js +type RefObject = { + value: T | null +}; + +interface React { + createRef(): RefObject; +} +``` + +`createRef` requires a explicit type annotation to ensure type correctness. + +After `componentDidMount` lifecycle `divRef.value` will be filled with element/component reference: + +```js +componentDidMount() { + this.divRef.value instanceof HTMLDivElement === true +} + +render() { + return ( +
+ {this.props.children} +
+ ); +} +``` + +# Drawbacks + +Why should we *not* do this? Please consider: + +- implementation cost, both in term of code size and complexity +- whether the proposed feature can be implemented in user space +- the impact on teaching people React +- integration of this feature with other existing and planned features +- cost of migrating existing React applications (is it a breaking change?) + +There are tradeoffs to choosing any path. Attempt to identify them here. + +# Alternatives + +What other designs have been considered? What is the impact of not doing this? + +# Adoption strategy + +If we implement this proposal, how will existing React developers adopt it? Is +this a breaking change? Can we write a codemod? Should we coordinate with +other projects or libraries? + +# How we teach this + +Callback and object refs solve different problems and crossing over in what they offer too - much like how components in React do currently. + +Someone would likely choose an object ref when they only care about binding the object reference to the component instance/state they're in. Then the ref can be used for caching, or being passed to another component/handler in a lifecycle event. + +Callback refs are great when someone likes to have find grain control over the node at the point where it gets provides and removed (like a subscription) to react to it. + +# Unresolved questions + +Optional, but suggested for first drafts. What parts of the design are still +TBD? From 0b0d5d96f02e23c24dfbac401aa0ca49a2c4eed7 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Thu, 25 Jan 2018 17:00:33 +0300 Subject: [PATCH 02/11] Tweak typo --- text/0000-new-create-ref.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-new-create-ref.md b/text/0000-new-create-ref.md index 58b47be6..f053779b 100644 --- a/text/0000-new-create-ref.md +++ b/text/0000-new-create-ref.md @@ -8,7 +8,7 @@ Replace string refs with similar functionality to how they currently work but wi # Basic example -The React.createRef() API will create an immutable object ref (where it's value is a mutable object referencing the actual ref). Accessing the ref value can be done via ref.value. An example of how this works is below: +The React.createRef() API will create an immutable object ref (where its value is a mutable object referencing the actual ref). Accessing the ref value can be done via ref.value. An example of how this works is below: ```js class MyComponent extends React.Component { From ea8070badca120808874a4e07694c4cc1d7151bd Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Fri, 9 Feb 2018 10:40:22 +0300 Subject: [PATCH 03/11] Improve motivation --- text/0000-new-create-ref.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/text/0000-new-create-ref.md b/text/0000-new-create-ref.md index f053779b..82a16167 100644 --- a/text/0000-new-create-ref.md +++ b/text/0000-new-create-ref.md @@ -26,6 +26,23 @@ class MyComponent extends React.Component { # Motivation +Strings refs bind to the React's component's `currentOwner` rather than the parent. That's something that isn't statical analysable and leads to most of bugs. + +class ComponentA { + render() { + return ( + // ref foo1 would bind to ComponentA +
+ { + // ref foo2 would bind to ComponentB + // even though it's all in ComponentA's render + () =>
+ } +
+ ); + } +} + This alternative API shouldn't provide any big real wins over callback refs - other than being a nice convenience feature. There might be some small wins in performance - as a common pattern is to assign a ref value in a closure created in the render phase of a component - this avoids that (even more so when a ref property is assigned to a non-existent component instance property). One benefit would be more correct Flow types. People tend to type refs incorrectly because Flow doesn't enforce uninitialized class properties correctly: From c709f3eb5b91fa44e11c6f79afd5a6e7890651c6 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Fri, 9 Feb 2018 17:11:41 +0300 Subject: [PATCH 04/11] Add drawback --- text/0000-new-create-ref.md | 62 ++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/text/0000-new-create-ref.md b/text/0000-new-create-ref.md index 82a16167..60198783 100644 --- a/text/0000-new-create-ref.md +++ b/text/0000-new-create-ref.md @@ -28,6 +28,7 @@ class MyComponent extends React.Component { Strings refs bind to the React's component's `currentOwner` rather than the parent. That's something that isn't statical analysable and leads to most of bugs. +```js class ComponentA { render() { return ( @@ -42,6 +43,7 @@ class ComponentA { ); } } +``` This alternative API shouldn't provide any big real wins over callback refs - other than being a nice convenience feature. There might be some small wins in performance - as a common pattern is to assign a ref value in a closure created in the render phase of a component - this avoids that (even more so when a ref property is assigned to a non-existent component instance property). @@ -87,15 +89,61 @@ render() { # Drawbacks -Why should we *not* do this? Please consider: +Callback refs are easier to use when child with ref and parent have different lifetime. With object ref it can be achieved with `componentDidUpdate` hook. As present in the example below callback refs are less verbose in this case. + +```js +class ComponentA extends React.Component { + setRef = (element) => { + if (element) { + // element is HTMLElement + // ref is created + } else { + // element is null + // ref is destroyed + // cached reference is required to stop listening events + } + } + + divRef = React.createRef(); + + componentDidMount() { + if (this.divRef.value) { + // ref is created + } + } + + componentDidUpdate() { + if (this.divRef.value) { + // ref is created + } else { + // ref is null + // should be used cached reference to stop listening events + } + } + + componentWillUnmount() { + if (this.divRef.value) { + // ref is HTMLElement, not removed yet + // cached reference is not required + } + } -- implementation cost, both in term of code size and complexity -- whether the proposed feature can be implemented in user space -- the impact on teaching people React -- integration of this feature with other existing and planned features -- cost of migrating existing React applications (is it a breaking change?) + render() { + if (this.props.enabled) { + return ( + <> +
+
+ + ); + } else { + return null; + } + } +} +``` -There are tradeoffs to choosing any path. Attempt to identify them here. +And still in most cases the same parent and ref lifetime makes code cleaner and simpler. # Alternatives From 7d0d3c4eb7f9510fb0c828a95facb8a838cc0d07 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Fri, 9 Feb 2018 17:41:53 +0300 Subject: [PATCH 05/11] Add alternatives and adoption strategy sections --- text/0000-new-create-ref.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/text/0000-new-create-ref.md b/text/0000-new-create-ref.md index 60198783..72c3a60c 100644 --- a/text/0000-new-create-ref.md +++ b/text/0000-new-create-ref.md @@ -147,13 +147,15 @@ And still in most cases the same parent and ref lifetime makes code cleaner and # Alternatives -What other designs have been considered? What is the impact of not doing this? +The common practice will be using ref objects (aka createRef), unless people have already been using callback refs and are happy with them. + +Also there's no point in changing from callback refs to createRef right now unless people see value in doing so, otherwise it's just tech debt with no real additional value other than maybe some perf wins from not creating closures in the render method. # Adoption strategy -If we implement this proposal, how will existing React developers adopt it? Is -this a breaking change? Can we write a codemod? Should we coordinate with -other projects or libraries? +Initially, we will release the object refs alongside the existing string refs as a minor update. Also this update will include `StrictMode` which enables deprecation messages for its subtree, so people be able to catch using string refs in their components and replace with the new api incrementally. + +String refs will be removed in upcoming major release, so there will be enought time for migration. # How we teach this From e249d45e79844a482c480721baec655defa22a21 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Fri, 9 Feb 2018 19:53:40 +0300 Subject: [PATCH 06/11] Improve motivation --- text/0000-new-create-ref.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/text/0000-new-create-ref.md b/text/0000-new-create-ref.md index 72c3a60c..791c8488 100644 --- a/text/0000-new-create-ref.md +++ b/text/0000-new-create-ref.md @@ -26,6 +26,10 @@ class MyComponent extends React.Component { # Motivation +The primary motivation is to encourage people to migrate off string refs. Callback refs meet some resistance because they are a bit harder to understand. The proposal introduces this API primarily for people who love string refs today. + +There's a few problems with them. + Strings refs bind to the React's component's `currentOwner` rather than the parent. That's something that isn't statical analysable and leads to most of bugs. ```js From 9ae1b73f3a4232c335cf94b5b501ce111c4e8802 Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Fri, 9 Feb 2018 20:12:22 +0300 Subject: [PATCH 07/11] Add more string ref problems --- text/0000-new-create-ref.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/text/0000-new-create-ref.md b/text/0000-new-create-ref.md index 791c8488..45db4aae 100644 --- a/text/0000-new-create-ref.md +++ b/text/0000-new-create-ref.md @@ -30,7 +30,7 @@ The primary motivation is to encourage people to migrate off string refs. Callba There's a few problems with them. -Strings refs bind to the React's component's `currentOwner` rather than the parent. That's something that isn't statical analysable and leads to most of bugs. +Strings refs bind to the React's component's `currentOwner` rather than the parent. This breaks "render prop" pattern. ```js class ComponentA { @@ -49,6 +49,12 @@ class ComponentA { } ``` +It is not statical analysable and leads to most of bugs. + +It requires that React keeps track of currently rendering component (since it can't guess this). This makes React a bit slower. + +Another problem is breaking with two instances of react if for some reason react packages weren't deduped. + This alternative API shouldn't provide any big real wins over callback refs - other than being a nice convenience feature. There might be some small wins in performance - as a common pattern is to assign a ref value in a closure created in the render phase of a component - this avoids that (even more so when a ref property is assigned to a non-existent component instance property). One benefit would be more correct Flow types. People tend to type refs incorrectly because Flow doesn't enforce uninitialized class properties correctly: From 3bfb6055a19d7032381e4f94b02fb9f6d7e2845d Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Fri, 9 Feb 2018 21:53:11 +0300 Subject: [PATCH 08/11] Tweak word --- text/0000-new-create-ref.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-new-create-ref.md b/text/0000-new-create-ref.md index 45db4aae..ef62b7f4 100644 --- a/text/0000-new-create-ref.md +++ b/text/0000-new-create-ref.md @@ -49,7 +49,7 @@ class ComponentA { } ``` -It is not statical analysable and leads to most of bugs. +It is not statically-analyzable and leads to most of bugs. It requires that React keeps track of currently rendering component (since it can't guess this). This makes React a bit slower. From 81a7d52e98c5b22032aaf2e65937b5bb4c5821be Mon Sep 17 00:00:00 2001 From: Bogdan Chadkin Date: Fri, 9 Feb 2018 22:17:45 +0300 Subject: [PATCH 09/11] Tweak word --- text/0000-new-create-ref.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0000-new-create-ref.md b/text/0000-new-create-ref.md index ef62b7f4..3a3bd14b 100644 --- a/text/0000-new-create-ref.md +++ b/text/0000-new-create-ref.md @@ -165,7 +165,7 @@ Also there's no point in changing from callback refs to createRef right now unle Initially, we will release the object refs alongside the existing string refs as a minor update. Also this update will include `StrictMode` which enables deprecation messages for its subtree, so people be able to catch using string refs in their components and replace with the new api incrementally. -String refs will be removed in upcoming major release, so there will be enought time for migration. +String refs will be removed in upcoming major release, so there will be enough time for migration. # How we teach this From f9c52ed58ca27b8483c62eabe3d2860af3ee152d Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sun, 11 Feb 2018 19:12:15 +0000 Subject: [PATCH 10/11] Reword the RFC --- text/0000-new-create-ref.md | 107 ++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 60 deletions(-) diff --git a/text/0000-new-create-ref.md b/text/0000-new-create-ref.md index 3a3bd14b..9ad5bb51 100644 --- a/text/0000-new-create-ref.md +++ b/text/0000-new-create-ref.md @@ -4,11 +4,20 @@ # Summary -Replace string refs with similar functionality to how they currently work but without many of the issues that string refs currently have (a list of them can be found at [#1373](/~https://github.com/facebook/react/issues/1373)). +Currently there are two ref APIs in React: string refs and callback refs. + +String refs are considered legacy due to [numerous issues](/~https://github.com/facebook/react/issues/1373) in their design. [Callback refs](https://reactjs.org/docs/refs-and-the-dom.html) don't share these deficiencies and were introduced to replace string refs. However, there is more ceremony around writing them, they have some [unintuitive caveats](https://reactjs.org/docs/refs-and-the-dom.html#caveats) and can be hard to teach. + +**The goal of this RFC is to introduce a new intuitive ref API. It is very similar in its "feel" to string refs, but doesn't suffer from their problems.** It is intentionally less poweful than the callback ref API. The plan is to: + +* Keep the callback ref API for more advanced use cases. +* Replace the string refs by the newly proposed "object" ref API. + +String refs would get deprecated in a minor release, and support for them would eventually get removed in an upcoming major release. # Basic example -The React.createRef() API will create an immutable object ref (where its value is a mutable object referencing the actual ref). Accessing the ref value can be done via ref.value. An example of how this works is below: +The `React.createRef()` API will create an immutable object ref (where its value is a mutable object referencing the actual ref). Accessing the ref value can be done via `ref.value`. An example of how this works is below: ```js class MyComponent extends React.Component { @@ -28,9 +37,9 @@ class MyComponent extends React.Component { The primary motivation is to encourage people to migrate off string refs. Callback refs meet some resistance because they are a bit harder to understand. The proposal introduces this API primarily for people who love string refs today. -There's a few problems with them. +### What's wrong with string refs? -Strings refs bind to the React's component's `currentOwner` rather than the parent. This breaks "render prop" pattern. +The main problem with string refs is that they require React to keep track of the "currently executing" component. However, that component doesn't always match where the string ref is defined. Consider the common "render prop" pattern: ```js class ComponentA { @@ -49,21 +58,20 @@ class ComponentA { } ``` -It is not statically-analyzable and leads to most of bugs. +This behavior is highly confusing. -It requires that React keeps track of currently rendering component (since it can't guess this). This makes React a bit slower. +There are other issues with string refs in practice: -Another problem is breaking with two instances of react if for some reason react packages weren't deduped. +* They cause bugs when [multiple React copies are on the same page](https://reactjs.org/warnings/refs-must-have-owner.html). +* They're not friendly to static typing. +* They aren't compatible with advanced compilation strategies that mangle class properties. +* They can't be [composed](/~https://github.com/ide/react-clone-referenced-element/). -This alternative API shouldn't provide any big real wins over callback refs - other than being a nice convenience feature. There might be some small wins in performance - as a common pattern is to assign a ref value in a closure created in the render phase of a component - this avoids that (even more so when a ref property is assigned to a non-existent component instance property). +### Don't we have callback refs for this? -One benefit would be more correct Flow types. People tend to type refs incorrectly because Flow doesn't enforce uninitialized class properties correctly: +Many people continue to favor string refs because writing callback refs requires more mental overhead (you have to think about a field *and* a function that sets it). They are especially inconvenient when you need an array of refs. Callback refs also have [unusual caveats](https://reactjs.org/docs/refs-and-the-dom.html#caveats) that are [often](/~https://github.com/facebook/react/issues/9328) [mistaken for a bug](/~https://github.com/facebook/react/issues/8619). -```js -class Foo { neverSet: boolean; } -let foo = new Foo(); -(foo.neverSet: boolean); // works -``` +Callback refs are, strictly saying, more powerful than either string refs or the proposed object refs. However, there is definitely a niche for a simpler, more convenient API for the majority of cases. There might be some small wins in performance too: commonly, ref value is assigned in a closure created in the render phase of a component. This API avoids that. Similarly, this avoids assigning to a non-existent component instance property, which also can cause deopts in JavaScript engines. # Detailed design @@ -99,53 +107,29 @@ render() { # Drawbacks -Callback refs are easier to use when child with ref and parent have different lifetime. With object ref it can be achieved with `componentDidUpdate` hook. As present in the example below callback refs are less verbose in this case. +Callback refs are easier to use when child with ref and parent have different lifetime. Consider this example: ```js -class ComponentA extends React.Component { - setRef = (element) => { - if (element) { - // element is HTMLElement - // ref is created +class CallbackRefExample extends React.Component { + // Callback refs offer a centralized place + // to perform side effects. + divRef = (div) => { + if (div) { + this.div = div; + this.div.addEventListener('foo', onFoo); } else { - // element is null - // ref is destroyed - // cached reference is required to stop listening events + this.div.removeEventListener('foo', onFoo); + this.div = null; } } - divRef = React.createRef(); - - componentDidMount() { - if (this.divRef.value) { - // ref is created - } - } - - componentDidUpdate() { - if (this.divRef.value) { - // ref is created - } else { - // ref is null - // should be used cached reference to stop listening events - } - } - - componentWillUnmount() { - if (this.divRef.value) { - // ref is HTMLElement, not removed yet - // cached reference is not required - } + onFoo = () => { + console.log('hello'); } render() { if (this.props.enabled) { - return ( - <> -
-
- - ); + return
; } else { return null; } @@ -153,29 +137,32 @@ class ComponentA extends React.Component { } ``` -And still in most cases the same parent and ref lifetime makes code cleaner and simpler. +We could write an equivalent example with object refs by using `componentDidMount` and `componentDidUpdate`. However, by the time `componentDidUpdate` fires, we have no way to access the previous ref value to remove the listener. So we'd have to "mirror" the ref value in a separate instance variable. This becomes very tedious and error-prone. + +The conclusion here is that for advanced use cases (such as side effects during ref attachment and detachment), callback refs are preferable. Still, in most cases this is not necessary, and object refs are sufficient. # Alternatives -The common practice will be using ref objects (aka createRef), unless people have already been using callback refs and are happy with them. +One potential alternative is to do nothing, and keep string refs and callback refs. The problem with keeping string refs is that they prevent some future work on React, including advanced compilation strategies. So we need a migration path off of them. + +Another potential alternative is to fully embrace callback refs but deprecate string refs. However, there is a lot of feedback suggesting that people struggle with understanding and using the callback ref API, and a simper option is necessary. -Also there's no point in changing from callback refs to createRef right now unless people see value in doing so, otherwise it's just tech debt with no real additional value other than maybe some perf wins from not creating closures in the render method. +The `createRef` API could potentially be implemented as a userland package. However, it would be a one-liner, and most people would not know about it. If we wanted everybody to use it, we might as well build it into React. # Adoption strategy -Initially, we will release the object refs alongside the existing string refs as a minor update. Also this update will include `StrictMode` which enables deprecation messages for its subtree, so people be able to catch using string refs in their components and replace with the new api incrementally. +Initially, we will release the object refs alongside the existing string and callback refs as a minor update. This update will also include a `` component which enables deprecation messages for its subtree. This lets people catch string refs usage in their components and replace with the new API (or callback refs, if they prefer) incrementally. -String refs will be removed in upcoming major release, so there will be enough time for migration. +String refs will be removed in an upcoming major release, so there will be enough time for migration. # How we teach this Callback and object refs solve different problems and crossing over in what they offer too - much like how components in React do currently. -Someone would likely choose an object ref when they only care about binding the object reference to the component instance/state they're in. Then the ref can be used for caching, or being passed to another component/handler in a lifecycle event. +In most cases people would choose an object ref. They could use it when they only care about accessing the object reference. The ref object can be passed around, or stored in an array. -Callback refs are great when someone likes to have find grain control over the node at the point where it gets provides and removed (like a subscription) to react to it. +Callback refs will be positioned as an advanced API. It is useful when someone likes to have fine-grained control over the node when it gets attached and detached, or to compose multiple refs. # Unresolved questions -Optional, but suggested for first drafts. What parts of the design are still -TBD? +None. From a8ccc5a517eb0932b23a5e2fe57fba8025b7369a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sun, 11 Feb 2018 19:20:21 +0000 Subject: [PATCH 11/11] Bookkeeping --- text/{0000-new-create-ref.md => 0017-new-create-ref.md} | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) rename text/{0000-new-create-ref.md => 0017-new-create-ref.md} (95%) diff --git a/text/0000-new-create-ref.md b/text/0017-new-create-ref.md similarity index 95% rename from text/0000-new-create-ref.md rename to text/0017-new-create-ref.md index 9ad5bb51..1cbfc116 100644 --- a/text/0000-new-create-ref.md +++ b/text/0017-new-create-ref.md @@ -1,6 +1,6 @@ -- Start Date: (fill me in with today's date, 2018-01-25) -- RFC PR: (leave this empty) -- React Issue: (leave this empty) +- Start Date: 2018-01-25 +- RFC PR: /~https://github.com/reactjs/rfcs/pull/17 +- React Issue: /~https://github.com/facebook/react/issues/10581 # Summary