Hydration DOM Fixture #13521

nhunzaker · 2018-08-30T22:30:34Z

It's still a little rough, but @gaearon nudged me to send this over anyway.

This PR adds a hydration fixture to the manual test fixtures. You can play around with it here:

http://react-hydration.surge.sh/hydration

Why we need this fixture

Testing client bugs related to hydrating markup is hard. Additionally, network conditions and browser plugins are difficult to test with JSDOM. Having an interactive tool makes it much easier to demonstrate problems and accidentally uncover new ones.

For example:

Change events do not fire on hydrated inputs

Visit http://react-hydration.surge.sh/hydration
Uncheck the "Hydrate" checkbox
Change the text in the right-hand panel
Click the "Hydrate" button
Observe that no change event occurs

React warns when 1Password adds meta data

Add the 1Password browser addon
Visit http://react-hydration.surge.sh/hydration
Uncheck the "Hydrate" checkbox
Change the text in the right-hand panel
Notice that 1Password adds data attributes to the input
Click the "Hydrate" button
Notice warnings in the console for data attributes that we have no control over. Should we warn in this case?

Things to do

nhunzaker · 2018-08-30T22:31:39Z

fixtures/dom/src/components/fixtures/hydration/index.js

+import './hydration.css';
+import {SAMPLE_CODE} from './data';
+import {LiveProvider, LiveEditor, LiveError} from 'react-live';
+import * as buble from 'buble';


Buble is the best!

Actually... spoke too soon (buble is still great) it turns out I needed Babel. Buble uses some __proto__ tricks that break in IE9/10.

To get IE9 support, I ended up pulling in babel/standalone with a few polyfills.

nhunzaker · 2018-08-30T22:33:36Z

packages/react-dom/src/client/ReactDOMFiberComponent.js

@@ -167,7 +167,7 @@ if (__DEV__) {
    attributeNames.forEach(function(name) {
      names.push(name);
    });
-    warningWithoutStack(false, 'Extra attributes from the server: %s', names);
+    warningWithoutStack(false, 'Extra attributes from the server: %s', names.join(', '));


Reverting this. This fixture caught this bug while I was working on it. I sent out a PR here:
#13385

nhunzaker · 2018-09-06T15:07:48Z

I believe this is ready for review. I've also verified that it works down to React 14.8 on IE9/10, Firefox 52, Safari 7.1, and Chrome 49 (but also the latest Firefox and Chrome)

pull-bot · 2018-09-06T15:16:04Z

Details of bundled changes.

Comparing: 8a8d973...81dcf6c

schedule

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
schedule.development.js	n/a	n/a	0 B	19.17 KB	0 B	5.74 KB	UMD_DEV
schedule.production.min.js	n/a	n/a	0 B	3.16 KB	0 B	1.53 KB	UMD_PROD

Generated by 🚫 dangerJS

nhunzaker · 2018-09-06T22:38:31Z

Well shoot... for some reason CodeMirror's layout messes up on the production build. I promise this looks okay:

I'll figure it out.

nhunzaker · 2018-09-06T22:56:42Z

Easiest thing is just to set a fixed height.

philipp-spiess

Awesome! I think it makes sense to have such a fixture. I also believe it's fine to add babel if we don't have any other choice.

Everything worked fine for me locally when using Chrome. 👍

Design Review

We should make the container bigger at least for the hydration fixture. Ideally we can print 80 characters in the editor without needing to scroll horizontally. Is this doable?

In addition to that, we should also use a bigger width for the error message when babel can't compile. There should also be <br>s between the individual lines if that isn't too much work.

I'm also not a super huge fan of the codemirror theme but since you've pushed the PR you can pick whatever you want 🙂

philipp-spiess · 2018-09-09T15:46:05Z

fixtures/dom/public/renderer.js

+  var renders = 0;
+  var failed = false;
+
+  function query(key) {


I was a bit confused about what we're querying here. Maybe getQueryParam() makes it less confusing? 🙂

philipp-spiess · 2018-09-09T15:53:14Z

fixtures/dom/src/components/fixtures/hydration/codemirror-paraiso-dark.css

+    Author:     Jan T. Sott
+
+    Color scheme by Jan T. Sott (https://github.com/idleberg/Paraiso-CodeMirror)
+    Inspired by the art of Rubens LP (http://www.rubenslp.com.br)


License looks 👍 But we need to include the copyright notice I believe (see here).

Seems like the GitHub files already added this 🙂

Totally. I updated this doc-header to include the information:
https://github.com/facebook/react/pull/13521/files#diff-b9d951f33318d5b9ee19bff3b7ad625eR4

philipp-spiess · 2018-09-09T16:25:45Z

fixtures/dom/public/renderer.js

+  }
+
+  function injectFixture(src) {
+    window.Fixture = undefined;


Can we put the Fixture into the closure of this file instead of the global namespace? I was confused where this was set when in fact it is very easy and all in the same file 🙂

Yup. See d5a4acd

philipp-spiess · 2018-09-09T16:38:01Z

fixtures/dom/src/components/fixtures/hydration/index.js

+                checked={hydrate}
+                onChange={this.setCheckbox}
+              />
+              Hydrate


I was confused about this checkbox. Maybe "Hydrate on reload" or "Auto Hydrate" makes it clearer?

Especially when this checkbox is unchecked you see a button with the same name.

Great idea! See 437c45b

This prevents a bug where Chrome reports `Array(n)` where `n` is the size of the array.

This commit adds a homepage to the DOM fixtures that includes browser testing information and asynchronously loads fixtures. This should make it easier to find DOM testing information and keep the payload size in check as we add more components to the fixtures.

This commit adds a first pass at a fixture that makes it easier to debug the process of hydrating static markup. This is not complete: 1. It needs to be verified across multiple browsers 2. It needs to render with the current version of react Still, it at least demonstrates the idea. A fixture like this will also be helpful for debugging change events for hydrated inputs, which presently do not fire if the user changes an input's text before hydration.

This commit makes a few tweaks to support older browsers and updates the code transition process to use window.postMessage. This avoids loading React on every single change.

For clarity, and so that the Fixture component does not need to be assigned to the window, this commit changes the way code is executed such that it evaluates using a Function constructor.

This commit extends the hydration fixture such that it takes up the full screen view. To accomplish this, the container that wraps all fixtures has been moved to the FixtureSet component, utilized by all other fixtures.

This commit fixes an issue where CodeMirror wouldn't layout correctly in production builds because the editor executes before the stylesheet loaded. CodeMirror needs layout information, and was rendering off-screen without correct CSS layout measurements.

nhunzaker · 2018-09-09T20:13:28Z

Okay! I got a little carried away, but I think I've made a lot of improvements:

http://react-hydration.surge.sh/hydration

I still need to do some browser testing, but over all I think I was able to get the design changes while simplifying the layout. I'm pretty happy.

nhunzaker · 2018-09-09T20:15:58Z

The error message is now collapsable too:

nhunzaker · 2018-09-09T20:16:52Z

fixtures/dom/public/renderer.js

+  }
+
+  function injectFixture(src) {
+    Fixture = new Function(src + '\nreturn Fixture;')();


@philipp-spiess great call here. I was evaluating this in the context of the window, but I'm pretty sure we can keep it local just using a Function constructor.

philipp-spiess · 2018-09-09T22:00:11Z

Oh wow, this looks really cool now 😎

I tested IE9 and Safari 7.1. In both browsers, the error message does not appear to work 😕 The error is different though.

I have no idea if it worked before your changes. Latest Chrome, Firefox, and Safari work great!

This commit fixes an error in Safari 7.1 where Chalk highlighted Babel errors caused a crash when setPrototypeOf was called within the library. This is also an issue on IE9, however this fix does not resolve issues in that browser.

- Reverts highlighting change. Polyfilling Safari 7.1 is sufficient - Do not render a details tag in IE9

nhunzaker · 2018-09-10T17:37:53Z

@philipp-spiess Thanks! It looks like this happens when Babel highlights errors. I was able to resolve the issue in Safari 7 by adding the setPrototypeOf polyfill. This will not work for IE9.

I'm not really sure what to do in IE9 short of fixing these lines in Chalk:
https://github.com/chalk/chalk/blob/a6ad9454525c0af602931049155cc51eccc72bb4/index.js#L39-L40

I think this is probably fine for now. It only affects IE9.

nhunzaker · 2018-09-10T21:04:04Z

Thanks for the fantastic review, @philipp-spiess!

* Add home component. Async load fixtures. This commit adds a homepage to the DOM fixtures that includes browser testing information and asynchronously loads fixtures. This should make it easier to find DOM testing information and keep the payload size in check as we add more components to the fixtures. * Adds experimental hydration fixture This commit adds a first pass at a fixture that makes it easier to debug the process of hydrating static markup. This is not complete: 1. It needs to be verified across multiple browsers 2. It needs to render with the current version of react Still, it at least demonstrates the idea. A fixture like this will also be helpful for debugging change events for hydrated inputs, which presently do not fire if the user changes an input's text before hydration. * Tweak select width * Manually join extra attributes in warning This prevents a bug where Chrome reports `Array(n)` where `n` is the size of the array. * Transform with buble * Eliminate dependencies * Pull in react-live for better editing * Handle encoding errors, pass react version * Load the correct version of React * Tweaks * Revert style change * Revert warning update * Properly handle script errors. Fix dom-server CDN loading * Fix 15.x releases * Use postMessage to reduce latency, support older browsers This commit makes a few tweaks to support older browsers and updates the code transition process to use window.postMessage. This avoids loading React on every single change. * Fix fixture renamespacing bug * Gracefully fallback to textarea in React 14 * Replace buble with babel, react-live with codemirror * Simplify layout to resolve production code-mirror issues * Tweak height rules for code-mirror * Update theme to paraiso * Format Code.js * Adjust viewport to fix CodeMirror resize issue in production build * Eliminate react-code-mirror * Improve error state. Make full stack collapsable * Add link to license in codemirror stylesheet * Make code example more concise * Replace "Hydrate" with "Auto-hydrate" for clarity * Remove border below hydration header * Rename query function in render.js * Use Function(code) to evaluate hydration fixture For clarity, and so that the Fixture component does not need to be assigned to the window, this commit changes the way code is executed such that it evaluates using a Function constructor. * Extend hydration fixture to fill width. Design adjustments This commit extends the hydration fixture such that it takes up the full screen view. To accomplish this, the container that wraps all fixtures has been moved to the FixtureSet component, utilized by all other fixtures. * Improve error scroll state * Lazy load CodeMirror together before executing This commit fixes an issue where CodeMirror wouldn't layout correctly in production builds because the editor executes before the stylesheet loaded. CodeMirror needs layout information, and was rendering off-screen without correct CSS layout measurements. * Fix indentation on error message * Do not highlight errors from Babel. Add setPrototypeOf polyfill This commit fixes an error in Safari 7.1 where Chalk highlighted Babel errors caused a crash when setPrototypeOf was called within the library. This is also an issue on IE9, however this fix does not resolve issues in that browser. * Increase resilience to bad errors in Hydration fixture - Reverts highlighting change. Polyfilling Safari 7.1 is sufficient - Do not render a details tag in IE9

facebook-github-bot added the CLA Signed label Aug 30, 2018

nhunzaker commented Aug 30, 2018

View reviewed changes

nhunzaker force-pushed the hydration-fixture branch from d3da7b9 to b3ce273 Compare September 6, 2018 15:03

nhunzaker force-pushed the hydration-fixture branch from bc2b3ae to 95854b8 Compare September 6, 2018 23:40

nhunzaker requested a review from philipp-spiess September 7, 2018 15:16

philipp-spiess approved these changes Sep 9, 2018

View reviewed changes

nhunzaker added 19 commits September 9, 2018 10:33

Manually join extra attributes in warning

de6ea59

This prevents a bug where Chrome reports `Array(n)` where `n` is the size of the array.

Tweak select width

1546840

Transform with buble

91cd8c2

Eliminate dependencies

ce9a2b2

Pull in react-live for better editing

546cb07

Handle encoding errors, pass react version

9dad4a7

Load the correct version of React

0c88335

Tweaks

50856cc

Revert style change

05f3042

Revert warning update

ad5c392

Properly handle script errors. Fix dom-server CDN loading

7284c87

Fix 15.x releases

046a27f

Use postMessage to reduce latency, support older browsers

eac9900

This commit makes a few tweaks to support older browsers and updates the code transition process to use window.postMessage. This avoids loading React on every single change.

Fix fixture renamespacing bug

2e69dc5

Gracefully fallback to textarea in React 14

be904d1

Replace buble with babel, react-live with codemirror

88b1dd8

Simplify layout to resolve production code-mirror issues

84ac939

nhunzaker added 15 commits September 9, 2018 10:33

Tweak height rules for code-mirror

b4ed966

Update theme to paraiso

8a40c04

Format Code.js

17a957d

Adjust viewport to fix CodeMirror resize issue in production build

9d2b4a9

Eliminate react-code-mirror

6cd0e2e

Improve error state. Make full stack collapsable

ecb1ea8

Add link to license in codemirror stylesheet

4b50198

Make code example more concise

3bf211e

Replace "Hydrate" with "Auto-hydrate" for clarity

437c45b

Remove border below hydration header

aa321e2

Rename query function in render.js

565ac72

Use Function(code) to evaluate hydration fixture

d5a4acd

For clarity, and so that the Fixture component does not need to be assigned to the window, this commit changes the way code is executed such that it evaluates using a Function constructor.

Extend hydration fixture to fill width. Design adjustments

ec2e1a2

This commit extends the hydration fixture such that it takes up the full screen view. To accomplish this, the container that wraps all fixtures has been moved to the FixtureSet component, utilized by all other fixtures.

Improve error scroll state

c7d4738

nhunzaker force-pushed the hydration-fixture branch from 95854b8 to e79cd06 Compare September 9, 2018 20:14

nhunzaker commented Sep 9, 2018

View reviewed changes

Fix indentation on error message

bccb786

nhunzaker added 2 commits September 10, 2018 10:14

Increase resilience to bad errors in Hydration fixture

81dcf6c

- Reverts highlighting change. Polyfilling Safari 7.1 is sufficient - Do not render a details tag in IE9

nhunzaker merged commit f6fb03e into facebook:master Sep 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hydration DOM Fixture #13521

Hydration DOM Fixture #13521

nhunzaker commented Aug 30, 2018 •

edited

Loading

nhunzaker Aug 30, 2018

nhunzaker Sep 6, 2018

nhunzaker Aug 30, 2018

nhunzaker commented Sep 6, 2018

pull-bot commented Sep 6, 2018 •

edited

Loading

schedule

nhunzaker commented Sep 6, 2018

nhunzaker commented Sep 6, 2018

philipp-spiess left a comment

philipp-spiess Sep 9, 2018

philipp-spiess Sep 9, 2018

nhunzaker Sep 9, 2018

philipp-spiess Sep 9, 2018

nhunzaker Sep 9, 2018

philipp-spiess Sep 9, 2018

nhunzaker Sep 9, 2018

nhunzaker commented Sep 9, 2018

nhunzaker commented Sep 9, 2018 •

edited

Loading

nhunzaker Sep 9, 2018

philipp-spiess commented Sep 9, 2018

nhunzaker commented Sep 10, 2018 •

edited

Loading

nhunzaker commented Sep 10, 2018

Hydration DOM Fixture #13521

Hydration DOM Fixture #13521

Conversation

nhunzaker commented Aug 30, 2018 • edited Loading

Why we need this fixture

Change events do not fire on hydrated inputs

React warns when 1Password adds meta data

Things to do

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nhunzaker commented Sep 6, 2018

pull-bot commented Sep 6, 2018 • edited Loading

schedule

nhunzaker commented Sep 6, 2018

nhunzaker commented Sep 6, 2018

philipp-spiess left a comment

Choose a reason for hiding this comment

Design Review

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

nhunzaker commented Sep 9, 2018

nhunzaker commented Sep 9, 2018 • edited Loading

Choose a reason for hiding this comment

philipp-spiess commented Sep 9, 2018

nhunzaker commented Sep 10, 2018 • edited Loading

nhunzaker commented Sep 10, 2018

nhunzaker commented Aug 30, 2018 •

edited

Loading

pull-bot commented Sep 6, 2018 •

edited

Loading

nhunzaker commented Sep 9, 2018 •

edited

Loading

nhunzaker commented Sep 10, 2018 •

edited

Loading