Skip to content
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

Refactor enzyme to use Adapters, initial React 16 support #1007

Merged
merged 23 commits into from
Aug 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"new-cap": [2, { "capIsNewExceptions": ["AND"] }],
"react/jsx-pascal-case": [2, { "allowAllCaps": true }],
"react/no-find-dom-node": 1,
"import/first": 0,
"no-underscore-dangle": [2, {
"allowAfterThis": true,
"allow": [
Expand Down
7 changes: 7 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,22 @@ matrix:
env: KARMA=true REACT=15.4
- node_js: "6"
env: KARMA=true REACT=15
- node_js: "6"
env: KARMA=true REACT=16
allow_failures:
- node_js: "6"
env: EXAMPLE=react-native
- node_js: "6"
env: EXAMPLE=mocha
- node_js: "6"
env: EXAMPLE=karma
- node_js: "6"
env: EXAMPLE=karma-webpack
- node_js: "6"
env: EXAMPLE=jest
env:
- REACT=0.13
- REACT=0.14
- REACT=15.4
- REACT=15
- REACT=16
1 change: 1 addition & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,5 @@
* [Selectors](/docs/api/selector.md)
* [Change Log](/CHANGELOG.md)
* [Future](/docs/future.md)
* [Adapter & Compatibility Proposal](/docs/future/compatibility.md)
* [Contributing Guide](/CONTRIBUTING.md)
5 changes: 5 additions & 0 deletions docs/future.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ There are several things we'd like to address with Enzyme that often get asked.
of projects that we plan on addressing in the near future:


### Enzyme Adapter & Compatibility

[See the full proposal](future/compatibility.md)


#### Improved CSS Selector Support

Currently, "hierarchical" CSS selectors are not supported in Enzyme. That means that the only CSS
Expand Down
184 changes: 184 additions & 0 deletions docs/future/compatibility.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
# Enzyme Adapter & Compatibility Proposal


## Motivation

This proposal is attempting to address a handful of pain points that Enzyme has been
subject to for quite a while. This proposal has resulted mostly [#715](https://github.com/airbnb/enzyme/issues/715),
Copy link
Collaborator

Choose a reason for hiding this comment

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

the second sentence seems to be missing some word somewhere, maybe "from" after "mostly"

Copy link
Member

Choose a reason for hiding this comment

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

bump

and a resulting discussion among core maintainers of this project.

The desired results of this proposal are the following:

1. Cleaner code, easier maintenance, less bug prone.

By standardizing on a single tree specification, the implementation of Enzyme would no longer have
to take into account the matrix of supported structures and nuanced differences between different
versions of React, as well as to some extent the differences between `mount` and `shallow`.

2. Additional libraries can provide compatible adapters
Copy link
Member

Choose a reason for hiding this comment

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

(nbd but you can use 1. for all of these, and then reordering won't require a diff on the entire list)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

true. i usually avoid this because when reading it in plain text it gets confusing


React API-compatible libraries such as `preact` and `inferno` would be able to provide adapters to Enzyme
for their corresponding libraries, and be able to take full advantage of Enzyme's APIs.

3. Better user experience (ie, bundlers won't complain about missing deps)

Enzyme has had a long-standing issue with static-analysis bundlers such as Webpack and Browserify because
of our usage of internal React APIs. With this change, this would be minimized if not removed entirely,
since these things can be localized into the adapter modules, and users will only install the ones they need.

Additionally, we can even attempt to remove the use of internal react APIs by lobbying for react-maintained packages
such as `react-test-renderer` to utilize the React Standard Tree (RST) format (details below).

4. Standardization and interopability with other tools

If we can agree on the tree format (specified below as "React Standard Tree"), other tools can start to use and
understand this format as well. Standardization is a good thing, and could allow tools to be built that maybe
don't even exist yet.


## Proposal


### React Standard Tree (RST)

This proposal hinges on a standard tree specification. Keep in mind that this tree needs to account for more
than what is currently satisfied by the output of something like `react-test-renderer`, which is currently
only outputting the "host" nodes (ie, HTML elements). We need a tree format that allows for expressing a full
react component tree, including composite components.

```
// Strings and Numbers are rendered as literals.
type LiteralValue = string | number

// A "node" in an RST is either a LiteralValue, or an RSTNode
type Node = LiteralValue | RSTNode

// if node.type
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does this mean?

type RenderedNode = RSTNode | [Node]

type SourceLocation = {|
fileName: string
lineNumber: number
|}

type NodeType = 'class' | 'function' | 'host';

// An RSTNode has this specific shape
type RSTNode = {|
// Either a string or a function. A string is considered a "host" node, and
// a function would be a composite component. It would be the component constructor or
// an SFC in the case of a function.
type: string | function;

// This node's type
nodeType: NodeType;

// The props object passed to the node, which will include `children` in its raw form,
// exactly as it was passed to the component.
props: object;

// The backing instance to the node. Can be null in the case of "host" nodes and SFCs.
// Enzyme will expect instances to have the _public interface_ of a React Component, as would
// be expected in the corresponding React release returned by `getTargetVersion` of the
// renderer. Alternative React libraries can choose to provide an object here that implements
// the same interface, and Enzyme functionality that uses this will continue to work (An example
// of this would be the `setState()` prototype method).
instance: ComponentInstance?;

// For a given node, this corresponds roughly to the result of the `render` function with the
// provided props, but transformed into an RST. For "host" nodes, this will always be `null` or
// an Array. For "composite" nodes, this will always be `null` or an `RSTNode`.
rendered: RenderedNode?;

// an optional property with source information (useful in debug messages) that would be provided
// by this babel transform: https://babeljs.io/docs/plugins/transform-react-jsx-source/
__source?: SourceLocation;
|}
```

### Enzyme Adapter Protocol

**Definitions:**

An `Element` is considered to be whatever data structure is returned by the JSX pragma being used. In the
react case, this would be the data structure returned from `React.createElement`


```
type RendererOptions = {
// An optional predicate function that takes in an `Element` and returns
// whether or not the underlying Renderer should treat it as a "Host" node
// or not. This function should only be called with elements that are
// not required to be considered "host" nodes (ie, with a string `type`),
// so the default implementation of `isHost` is just a function that returns
// false.
?isHost(Element): boolean;
}

type EnzymeAdapter = {
// This is a method that will return a semver version string for the _react_ version that
Copy link
Collaborator

Choose a reason for hiding this comment

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

_react_ -> `react`

// it expects enzyme to target. This will allow enzyme to know what to expect in the `instance`
// that it finds on an RSTNode, as well as intelligently toggle behavior across react versions
// etc. For react adapters, this will likely just be `() => React.Version`, but for other
// adapters for libraries like inferno or preact, it will allow those libraries to specify
// a version of the API that they are committing to.
getTargetApiVersion(): string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we just required that adapters be explicitly configured, would we need this getTargetApiVersion API at all? What happens if a React-like adapter (Preact, Inferno) exists for a version that collides with a React version? e.g., Preact has a 15.5 release.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah. i think you're right. I think I will get rid of this, and let node semver declarations take care of the rest.


// Provided a bag of options, return an `EnzymeRenderer`. Some options can be implementation
// specific, like `attach` etc. for React, but not part of this interface explicitly.
createRenderer(?options: RendererOptions): EnzymeRenderer;

// converts an RSTNode to the corresponding JSX Pragma Element. This will be needed
// in order to implement the `Wrapper.mount()` and `Wrapper.shallow()` methods, but should
Copy link
Collaborator

Choose a reason for hiding this comment

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

ReactWrapper.prototype.mount(), ShallowWrapper.prototype.shallow()

Copy link
Collaborator

Choose a reason for hiding this comment

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

the only purpose of this option is to support using enzyme with libraries that use a different Element representation? are we sure everything else in the enzyme internals is agnostic to this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, i'm not sure... but when someone tries, hopefully they will file a bug :)

// be pretty straightforward for people to implement.
nodeToElement(RSTNode): Element;
}

type EnzymeRenderer = {
// both initial render and updates for the renderer.
render(Element): void;

// retrieve a frozen-in-time copy of the RST.
getNode(): RSTNode?;
}
```


### Using different adapters with Enzyme

At the top level, Enzyme would expose a `configure` method, which would allow for an `adapter`
option to be specified and globally configure Enzyme's adapter preference:

```
import Enzyme from 'enzyme';
import ThirdPartyEnzymeAdapter from 'third-party-enzyme-adapter';

Enzyme.configure({ adapter: ThirdPartyEnzymeAdapter });

```

Additionally, each wrapper Enzyme exposes will allow for an overriding `adapter` option that will use a
Copy link
Collaborator

Choose a reason for hiding this comment

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

in this sentence we're using wrapper to mean 2 different things, first it means the factory for wrappers, 2nd it means a thing returned by one the factories, can we rename one of them, or both?

given adapter for just that wrapper:

```
import { shallow } from 'enzyme';
import ThirdPartyEnzymeAdapter from 'third-party-enzyme-adapter';

shallow(<Foo />, { adapter: ThirdPartyEnzymeAdapter });
```

Enzyme will build adapters for all major versions of React since React 0.13, though will deprecate
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are we maintaining compatibility with 4 major react versions at the same time?

Copy link
Collaborator

Choose a reason for hiding this comment

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

or maybe this will be released before we release support for react 16?

Copy link
Member

Choose a reason for hiding this comment

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

One of the main benefits of the adapter pattern is that we don't have to ever drop support for a React major version, since the cost of maintaining the adapter is so minimal.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point, I agree

adapters as usage of a particular major version fades.

```
import React13Adapter from 'enzyme-adapter-react-13';
import React14Adapter from 'enzyme-adapter-react-14';
import React15Adapter from 'enzyme-adapter-react-15';
// ...
```

### Validation

Enzyme will provide an `validate(node): Error?` method that will traverse down a provided `RSTNode` and
return an `Error` if any deviations from the spec are encountered, and `null` otherwise. This will
provide a way for implementors of the adapters to determine whether or not they are in compliance or not.
Loading