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

Improve hydration fixture, support older versions of React #14118

Merged
merged 5 commits into from Mar 13, 2019

Conversation

nhunzaker
Copy link
Contributor

@nhunzaker nhunzaker commented Nov 6, 2018

What

This PR makes some improvements to the DOM test fixtures to make it easier to test older versions of React.

Here's a link:
http://react-hydration-fixture.surge.sh/hydration

Why

After trying to figure out the origin of a few DOM bugs recently, it gave me some motivation to figure out how we can make that faster. I wanted to quickly test when a bug started or stopped, but discovered my only option was a lot of npm install + build/test across commits.

I think we can do that better, and I think the hydration fixture provides a good foundation for that. If we can figure out a way to host commit-level builds, we could very quickly pass through a range of commits to triage an issue in the browser.

Updates

  • Updating the App component to use a class instead of a function lets us support all fixtures down to 0.11
  • For really old versions, I generalized the React version picker and added it to the hydration fixture. It can now override the current version of React and supports down to React 0.4.0
  • Create a wrapper around findDOMNode for older React
  • A small UX fix for the error message on the Hydration Fixture
  • Additional documentation in the fixtures as to when key API changes happened.

I don't want to support all the way back to 0.4.0 for all test fixtures, but I think having some utilities to do it quickly would be useful.


image

@sophiebits
Copy link
Collaborator

sophiebits commented Nov 6, 2018

One idea I've thought about that you may be interested in investigating: we may be able to use Metro, the packager used by RN, to serve bundles of React dynamically from our repo. A lot of its configuration overlaps with Jest's which is why I'm hopeful this could work.

Basically we could end up with a way to start a server that vends built React bundles that fixtures could then depend on – they would not be exactly identical to our Rollup process, but they would likely be much faster and could support incremental building. (Metro has very fast reload times – so when switching between commits only a few files would rebuild, much like yarn test MyTestFile --watch.)

if you're interested in investigating this here is one example of using Metro on the web by one of the members of the team here at FB: https://github.com/rafeca/metro-sample-app.

(A particular advantage of using Metro rather than just caching old builds would be that it would also be useful for developing locally.)

@nhunzaker
Copy link
Contributor Author

That's very interesting. I can definitely look into this!

Fixes an issue where the hydration fixture would try to load in
ReactDOMServer below version 14. In version 13, string markup methods
exist on the React namespace.
This was breaking React 0.13.0.
This commit fixes an issue where the Hydration DOM fixture was
unusable in React 0.13.0 or lower because of newer API usage.

It fixes that by avoiding the use of refs to get the textarea
reference in the code editor component, using various versions of
findDOMNode as required.
If an error showed for the hydration fixture, a detail element was
used even if no additional lines could display. In that case, this
commit changes the component such that it returns a div.
This commit adds support for versions 0.4.0 of React and higher for
the hydration fixture.

The DOM test fixtures themselves do not support down to React 0.4.0,
which would be exhaustive. Instead, the Hydration fixture can pick a
version to use for its own purposes. By default, this is the version
of React used by the fixtures.

In the process of doing this, I had to make some updates to the
renderer.html document associated with the hydration fixture, and I've
added some comments to better document the history of API changes.
@nhunzaker
Copy link
Contributor Author

@philipp-spiess, @jquense, @aweary: it's been a while, but I wanted to see if we could land this on the DOM fixtures. I still plan to see if we could use the metro bundler for quick commit-level builds, however I'd like to close this out if possible and I think these are useful improvements.

Copy link
Contributor

@philipp-spiess philipp-spiess left a comment

Choose a reason for hiding this comment

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

Nice!

Sorry for the super long waiting time. 😐

@nhunzaker
Copy link
Contributor Author

@philipp-spiess no worries. Thanks ❤️

@nhunzaker nhunzaker merged commit 679402a into facebook:master Mar 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants