Skip to content

Bundle react-test-renderer with AVA #1175

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

Closed
vadimdemedes opened this issue Jan 9, 2017 · 10 comments
Closed

Bundle react-test-renderer with AVA #1175

vadimdemedes opened this issue Jan 9, 2017 · 10 comments
Labels

Comments

@vadimdemedes
Copy link
Contributor

vadimdemedes commented Jan 9, 2017

Is there any reason why user has to manually import react-test-renderer and render their React tree "by hand"?

import test from 'ava';
import React from 'react';
import renderer from 'react-test-renderer';

test('snapshots', t => {
  const tree = renderer.create(<h1>Test</h1>).toJSON();
  t.snapshot(tree);
});

We could bundle react-test-renderer and that test would become:

import test from 'ava';
import React from 'react';

test('snapshots', t => {
  const tree = <h1>Test</h1>;
  t.snapshot(tree);
});

...which is cleaner and nicer, imo.

Main reason for it is to also allow magic assert to do diffing on snapshots. Because trees in snapshots are not shallowly rendered (full trees are saved instead), we can't use react-element-to-jsx-string module, so we need react-test-renderer inside AVA either way.

What do you think?

@sindresorhus
Copy link
Member

Sounds like an easy win. I don't like how big its dependency tree is though. It brings with it 2.5MB of mostly unused dependencies...

@jfmengels
Copy link
Contributor

jfmengels commented Jan 11, 2017

What about projects that use JSX with another tool than React (Inferno, Preact, Snabbdom, ...)?

@vadimdemedes
Copy link
Contributor Author

Hmm, good point. Would be great if we had a universal render-to-string package, that would detect React/Preact/Inferno/etc vnodes and render them to string.

@vadimdemedes
Copy link
Contributor Author

Then t.jsxEqual() would be universal as well.

@vadimdemedes
Copy link
Contributor Author

vadimdemedes commented Jan 16, 2017

On the other hand though, t.jsxEqual(), diffs and syntax highlighting in magic assert are tied to React. Unfortunately we can't be "easily" universal at the moment.

Here are the possible routes we can go:

  1. Create packages to serialize library-dependent JSX trees into JSON, bundle them and support most popular frameworks out of the box

This way would be the most comfortable from the user's perspective, but the most time consuming and expensive in terms of maintenance for us.

  1. Remove React support and let the user handle JSX assertions

Least preferable way for both users and AVA, despite being the easiest solution. For diffs users would use libraries like chai-jsx.

  1. Leave t.jsxEquals() and make it accept JSON tree, like in t.snapshot()

This would make t.jsxEquals() compatible with every possible JSX-based library. I think this is best way to go. Developers can create similar libraries to react-test-renderer (converts JSX to JSON), which would allow AVA users to write tests with these libraries.

A shortcut could make it simpler to transform JSX for assertions:

import renderer from 'react-test-renderer';

const render = tree => renderer.create(tree).toJSON();

test('jsx', t => {
  t.jsxEqual(render(<HelloNewYork/>), render(<HelloSanFrancisco/>));
});

@novemberborn
Copy link
Member

Can we use a Babel plugin to solve this?

@vadimdemedes
Copy link
Contributor Author

vadimdemedes commented Jan 17, 2017

I don't think it can be reliably implemented, because we'd have to detect which library is imported (react/preact/inferno/etc) and import according react-test-renderer alternative (and I don't think there are any right now).

In my opinion, we could do just fine with the third option for a while. Minimal maintenance, universal compatibility.

@vadimdemedes vadimdemedes mentioned this issue Jan 17, 2017
24 tasks
@novemberborn
Copy link
Member

Rephrasing yesterday's comment, now that I actually know what I meant to say…:

Could we support Babel plugins that rewrite t.jsxEqual() to automatically apply the JSON conversion? That way (assuming you don't mix JSX dialects within the same project) you don't need to do it manually. So option three, on steroids.

@vadimdemedes
Copy link
Contributor Author

So steroids (babel plugins to transform JSX into JSON) are basically left for community to implement? Sure, why not? They can freely use any babel plugins in configuration.

@vadimdemedes
Copy link
Contributor Author

Closing this, since the issue is now resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants