Forward ref #1592

jquense · 2018-03-23T16:14:56Z

This is built on top the Context PR, i'll rebase once that is merged

Adds support for forwardRef to accompany the new changes.

The dependencies for these to work are getting fairly specific at this point, I’d like to bump the dep range for react-test-renderer, but that effectively bumps the peer deps for react, maybe worth splitting to a new adapter at this point?

It would be really great if we could use toTree from react-test-renderer directly…

Fixes #1604.

…ided, defer to its definition of a valid element type.

petegleeson · 2018-04-04T02:31:49Z

Hi mate can we rebase these changes with master now #1513 is merged?

The lack of forwardRef support is a bit of a blocker for us at the moment. I have time to work on this so please let me know if there is anything I can do.

jquense · 2018-04-04T14:41:45Z

CI failure looks like a travis stalled build

petegleeson

I have verified these changes fix our use-case for forwardRef. 👍

As you mention, I found the versions to be pretty fiddly. In our codebase, I got runtime errors after running yarn add enzyme enzyme-adapter-react-16. This was due to dependencies like react-test-renderer and enzyme-adapter-utils not being bumped to the latest. Running yarn upgrade enzyme enzyme-adapter-react-16 solved this problem for me.

Droeftoeter · 2018-04-06T18:25:03Z

packages/enzyme/src/Utils.js

 export const ITERATOR_SYMBOL = typeof Symbol === 'function' && Symbol.iterator;

+// TODO use react-is when a version is released with no peer dependency on React


The latest react-is release has your change in it and does not have react as peer dependency anymore. Could this be updated?

a) react-is must never be used in new places outside of a react adapter, since non-react adapters exist. b) react-is does not work on react 0.13.

So, all of these things need to be exposed via the adapter interface - instead of being part of enzyme.

I'm not sure what's actionable here. It' seems like you are saying we should update all adapters to provide a typeOf api?

Yes, i think that might be what's required :-/

if so, let's do that in a separate PR, so that when this one is rebased, it can only include the diffs to typeOf?

oof ok yeah i'll pull this out for now

azmenak · 2018-04-23T15:17:07Z

As @petegleeson mentioned, this PR seems to be a blocker for anyone using forwardRefs. Anything we can do to get this moving?

Seems the react-is changes are strait forward, is there any way to add those changes to this PR? Happy to make those edits if it helps.

sambokai · 2018-05-20T18:31:12Z

Seems like this is solved by: facebook/react#12725

janv · 2018-06-15T12:58:49Z

What's holding this back?

cyan33 · 2018-06-18T23:41:54Z

Does this PR fix the forwardRef issue and is good to merge? Much appreciated if this gets merged soon. 😄

nemoDreamer · 2018-06-27T13:59:46Z

Yup... I'm about to switch my tests that rely on this back to Facebook's TestRenderer... 😢

jquense · 2018-06-27T14:11:10Z

If anyone needs an interim solution i've released @monastic.panic/enzyme-adapter-react-16 folks can use

ljharb

Thanks - this needs a number of changes before it can land.

ljharb · 2018-06-29T22:54:11Z

packages/enzyme-adapter-utils/src/createMountWrapper.jsx

@@ -55,7 +60,7 @@ export default function createMountWrapper(node, options = {}) {
    }
  }
  WrapperComponent.propTypes = {
-    Component: PropTypes.oneOfType([PropTypes.func, PropTypes.string]).isRequired,
+    Component: PropTypes.oneOfType([PropTypes.func, PropTypes.string, specialType]).isRequired,


PropTypes.element?

its not an element right it's a component class, e..g element.type

$$typeof isn't in react 13 (which is why react-is doesn't work on it), so i'm not sure that the propType is correct.

o I see what you mean, PropTypes.element won't fire validate a Context.Consumer or Forward ref "class" or type correctly unfortunately. I guess we could switch on react version for the proptype and not include teh specialType below 16?

We could have multiple things here - but we might want to add a propType to each adapter for "a component" - that way react 0.13 can specify that the function has to be a class component, for example, and not an SFC.

isValidElementType that we added would be the the right check I think. We could pass that in and use it in a custom proptype

@jquense This may be helpful since I just figured this same prop type check out: facebook/prop-types#200 (comment)

(Eagerly awaiting this PR merge)

ljharb · 2018-06-29T22:56:42Z

packages/enzyme-test-suite/test/_helpers/react-compat.js

@@ -37,9 +38,16 @@ if (is('^16.3.0-0')) {
  createContext = null;
 }

+if (is('^16.3.0-0')) {
+  ({ forwardRef } = require('react'));


let's combine createContext with forwardRef, since they're in the same versions.

ljharb · 2018-06-29T22:58:11Z

packages/enzyme/src/Debug.js

@@ -66,6 +94,7 @@ function indentChildren(childrenStrs, indentLength) {

 export function debugNode(node, indentLength = 2, options = {}) {
  if (typeof node === 'string' || typeof node === 'number') return escape(node);
+  if (typeof node === 'function') return '[function child]';


why is this necessarily a function child?

I'm not sure what else to call a props.children that's a function. Perhaps cases also hit here, but i'm not sure what they are.

right but debugNode won't necessarily be called on a child.

fair, should we just call it [function] then?

ljharb · 2018-06-29T22:58:59Z

packages/enzyme/src/Utils.js

 export const ITERATOR_SYMBOL = typeof Symbol === 'function' && Symbol.iterator;

+// TODO use react-is when a version is released with no peer dependency on React


a) react-is must never be used in new places outside of a react adapter, since non-react adapters exist. b) react-is does not work on react 0.13.

So, all of these things need to be exposed via the adapter interface - instead of being part of enzyme.

ljharb · 2018-06-29T22:59:27Z

packages/enzyme/src/Utils.js

+export const Portal = hasSymbol ? Symbol.for('react.portal') : 0xeaca;
+export const StrictMode = hasSymbol ? Symbol.for('react.strict_mode') : 0xeacc;
+
+export function typeOf(node) {


this needs unit tests.

cyan33 · 2018-06-29T23:28:31Z

packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js

+    case ContextProvider:
+    case ContextConsumer:
+    case Mode:
+    case ForwardRef:


If we want to find the ForwardRef itself, we should add a node for it, not only its children here in this case.

cyan33 · 2018-06-29T23:32:32Z

To fix finding forwardRef itself, I had to change this function:

export function nodeMatchesObjectProps(node, props) {
  if (typeOf(node) === ForwardRef) {
    return isSubset(node.type, replaceUndefinedValues(props));
  }
  return isSubset(propsOfNode(node), replaceUndefinedValues(props));
}

and in the react-16-adapter, in the toTree method:

case ForwardRef:
    return {
        nodeType: 'forward_ref',
        type: node.type,
        props: { ...node.memoizedProps },
        rendered: childrenToTree(node.child),
     };

Not sure if it's decent but it fixes my issue.

jquense · 2018-06-30T00:32:08Z

Alright, as @cyan33 notes, the problem here is that we can't add support for ForwardRef selecting without some sort of brand check.

mount(<TreeWithForwardRef/>).find(ForwardRefComponent)

Doesn't work here because selectors are pretending to be agnostic, but are actually codifying the assumption that element types can only be functions or strings, which isn't true anymore for React. The simplest but dirty approach is to go with what I have here for typeof logic, which is honestly an extension of the current state of affairs, even if that SOA isn't the intended one. The alternative, as @ljharb notes, is to add a isValidElementType api to adapters and pass that through to the selectors, which is a much bigger refactor.

ljharb · 2018-06-30T05:31:28Z

I think adding that API to the adapters is a fine idea; let's just do it in a separate PR and rebase this after it's merged.

wmonk · 2018-07-20T09:32:55Z

Thanks for your work on this! Is there any capacity to get this moving forward? I've tried the patched packages but none of them seemed to work.

wmonk · 2018-08-16T10:12:06Z

🎉 is there an ETA for this getting published to npm?

ljharb · 2018-08-16T16:06:03Z

It was merged 9 hours ago; you asked 6 hours ago. Have some patience.

jquense · 2018-08-16T16:07:36Z

Thanks for finishing this up @ljharb!

quantizor · 2018-08-16T17:24:54Z

@ljharb We all want our golden goose and want it now! 😂

quantizor · 2018-08-20T06:17:43Z

@ljharb looks like there's been a few enzyme patch releases since this was merged. Could we get a react 16 adapter release also to start using this? 🙇

ljharb · 2018-08-20T06:18:50Z

@probablyup those were backports that don't include this (or other semver-minor) changes. This feature remains unreleased, but will be released when things are ready.

kenips · 2018-08-20T19:17:32Z

@ljharb I know this isn't typical release style but considering the huge volume here could you please please please consider cutting a beta, next, or 1.3.0.beta-1 for ease of testing this? Thank you <3

@jquense

- Add `displayNameOfNode`, `isValidElementType` (#1701, @jquense) - pass the adapter into `createMountWrapper` (#1592, @jquense) - update deps - [meta] ensure a license and readme is present in all packages when published

@ljharb

- Add pointer events support (#1753, @ljharb) - Add `displayNameOfNode`, `isValidElementType` (#1701, @jquense) - pass the adapter into `createMountWrapper` (#1592, @jquense) - update deps - [meta] ensure a license and readme is present in all packages when published

@jgzuke

- [New] Add Portal support (#1760, #1761, #1772, #1774, @jgzuke) - [New] Add pointer events support (#1753, @ljharb) - [New] Add `displayNameOfNode`, `isValidElementType` (#1701, @jquense) - [New] pass the adapter into `createMountWrapper` (#1592, @jquense) - [Fix] `shallow`: skip updates when nextState is `null` or `undefined` (#1785, @koba04) - update deps - [meta] ensure a license and readme is present in all packages when published

@jquense

- [New] Add forwardRef support (#1592, @jquense) - [New] Add Portal support (#1760, #1761, #1772, #1774, @jgzuke) - [New] Add pointer events support (#1753, @ljharb) - [New] Add `displayNameOfNode`, `isValidElementType` (#1701, @jquense) - [New] pass the adapter into `createMountWrapper` (#1592, @jquense) - [Fix] preemptively fix compat with React v16.4.3 (#1790, #1778, @gaearon, @aweary) - [Fix] `shallow`: skip updates when nextState is `null` or `undefined` (#1785, @koba04) - update deps - [meta] ensure a license and readme is present in all packages when published

@jquense

- [New] Add forwardRef support (#1592, @jquense) - [New] Add Portal support (#1760, #1761, #1772, #1774, @jgzuke) - [New] Add pointer events support (#1753, @ljharb) - [Fix] preemptively fix compat with React v16.4.3 (#1790, #1778, @gaearon, @aweary) - [Fix] `shallow`: prevent rerenders with PureComponents (#1786, @koba04) - [Fix] `shallow`: skip updates when nextState is `null` or `undefined` (#1785, @koba04) - [Fix] `shallow`: `setState` after `setProps` calls `componentWillReceiveProps` (#1779, @peanutenthusiast) - [Fix] `mount`/`shallow`: be stricter on the wrapper’s setState/setProps callback - [Fix] `shallow`/`mount`: improve error message when wrapping invalid elements (#1759, @jgzuke) - update deps - [Refactor] remove most uses of lodash - [meta] ensure a license and readme is present in all packages when published

ljharb · 2018-08-25T08:19:34Z

enzyme v3.5 is released: https://twitter.com/ljharb/status/1033267583372292098

kenips · 2018-08-25T19:52:14Z

Thank you @ljharb your great work is very much appreciated.

jquense added 2 commits March 23, 2018 12:11

[enzyme-adapter-react-*] pass the adapter into createMountWrapper

b396e5c

[enzyme-adapter-utils] createMountWrapper: when the adapter is prov…

ea9906f

…ided, defer to its definition of a valid element type.

jquense force-pushed the forward-ref branch from f79aec0 to ecb3fe2 Compare March 29, 2018 21:10

jquense force-pushed the forward-ref branch from ecb3fe2 to 9c6bcb2 Compare April 4, 2018 12:19

petegleeson approved these changes Apr 5, 2018

View reviewed changes

koba04 mentioned this pull request Apr 5, 2018

forwardRef: Enzyme Internal Error: unknown node with tag 14 #1604

Closed

10 tasks

petegleeson mentioned this pull request Apr 5, 2018

Complete React 16 support #1553

Open

41 tasks

Droeftoeter suggested changes Apr 6, 2018

View reviewed changes

petegleeson mentioned this pull request Apr 30, 2018

Enables finding nodes created by forwardRef jquense/enzyme#2

Open

abotteram mentioned this pull request May 1, 2018

Modal component WordPress/gutenberg#6261

Merged

4 tasks

cyan33 mentioned this pull request Jun 18, 2018

[WIP] Add a failed test on finding forwardRef component #1682

Closed

cyan33 mentioned this pull request Jun 20, 2018

Fix finding forwardRef itself #1684

Closed

nerrad mentioned this pull request Jun 27, 2018

WIP: Feature: Implement forwardRef in wp.element.createHigherOrderComponent WordPress/gutenberg#7557

Closed

10 tasks

ljharb requested changes Jun 29, 2018

View reviewed changes

cyan33 reviewed Jun 29, 2018

View reviewed changes

jquense mentioned this pull request Jul 2, 2018

Add additional type checks to adapters #1701

Merged

ljharb added this to Refs in React 16 Jul 5, 2018

aldeed mentioned this pull request Jul 16, 2018

feat: get all components from context reactioncommerce/reaction-component-library#100

Merged

ljharb added package: react adapter: 15 package: react adapter: 15.4 package: react adapter: 0.14 package: react adapter: 0.13 package: react adapter: 16.1 package: react adapter: 16.2 package: react adapter: 16.3 labels Aug 16, 2018

evanpurkhiser mentioned this pull request Aug 16, 2018

fix(forms): Correct ref and prop passing for textareas getsentry/sentry#9233

Merged

ericberens mentioned this pull request Aug 23, 2018

Enzyme Internal Error: unknown node with tag 14 rebassjs/grid#132

Closed

davit-y mentioned this pull request Aug 24, 2018

Fix TracePageHeader tests once new version of Enzyme is released jaegertracing/jaeger-ui#239

Closed

This was referenced Aug 25, 2018

Update enzyme in / from 3.1.0 to 3.5.0 pcardune/redux-firebase-mirror#198

Open

Update 2 dependencies from npm JetBrains/ring-ui#375

Merged

Update 2 dependencies from npm JetBrains/ring-ui#376

Closed

dependencies bot mentioned this pull request Aug 27, 2018

Update enzyme in / from 3.4.4 to 3.5.0 severest/retrobot#209

Closed

This was referenced Oct 1, 2018

Update enzyme in /modules/frontend from 3.3.0 to 3.6.0 project-accelerate/accelerate#265

Open

Update enzyme in / from 3.3.0 to 3.6.0 leonardofaria/leozera-ui#234

Closed

		export const ITERATOR_SYMBOL = typeof Symbol === 'function' && Symbol.iterator;

		// TODO use react-is when a version is released with no peer dependency on React

Forward ref #1592

Forward ref #1592

Conversation

jquense commented Mar 23, 2018 • edited by ljharb

petegleeson commented Apr 4, 2018

jquense commented Apr 4, 2018

petegleeson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azmenak commented Apr 23, 2018

sambokai commented May 20, 2018 • edited

janv commented Jun 15, 2018

cyan33 commented Jun 18, 2018

nemoDreamer commented Jun 27, 2018

jquense commented Jun 27, 2018

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyan33 commented Jun 29, 2018 • edited

jquense commented Jun 30, 2018

ljharb commented Jun 30, 2018

wmonk commented Jul 20, 2018

wmonk commented Aug 16, 2018

ljharb commented Aug 16, 2018

jquense commented Aug 16, 2018

quantizor commented Aug 16, 2018

quantizor commented Aug 20, 2018 • edited

ljharb commented Aug 20, 2018

kenips commented Aug 20, 2018

ljharb commented Aug 25, 2018

kenips commented Aug 25, 2018

jquense commented Mar 23, 2018 •

edited by ljharb

sambokai commented May 20, 2018 •

edited

cyan33 commented Jun 29, 2018 •

edited

quantizor commented Aug 20, 2018 •

edited