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

Add Flow Coverage to Renderer Fixture, or use a maintained renderer for tests #11220

Closed
sebmarkbage opened this issue Oct 13, 2017 · 4 comments
Closed

Comments

@sebmarkbage
Copy link
Collaborator

The renderer package contains a fixture that has to be updated every time we change APIs. However it is not covered by Flow nor by full test coverage so it is very tedious to make sure you get it exactly right.

I messed it up in #11213

Making changes to the API also means updating a lot of renderers at this point. It would be nice if the fixture could be built on top of the Noop renderer or something.

cc @iamdustan @gaearon

@iamdustan
Copy link
Contributor

I was wanting to try to generate a react-reconciler/index.js.flow file in the distribution as well so that:

  • consumers can get flow typechecking of userland renderers
  • run flow in the fixture file during release. If it fails we know there was a breaking API change.

@acdlite
Copy link
Collaborator

acdlite commented Oct 13, 2017

It would be nice if the fixture could be built on top of the Noop renderer or something.

👍

@iamdustan
Copy link
Contributor

use a maintained renderer for tests

I could look into this, too. The caveat is that it has to be a renderer with no internal dependencies (which noop has).

@gaearon
Copy link
Collaborator

gaearon commented Nov 28, 2017

#11683 solves this by running bundle tests (which I added) against the real reconciler bundles. (This means ReactNoop CommonJS build is now using the reconciler package.)

So I deleted the fixture.

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

No branches or pull requests

4 participants