From 3786a89298c1d972a02b946407240571a3eca202 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 11 Oct 2017 13:08:18 +0100 Subject: [PATCH 1/4] Keep autoFocus attribute in the DOM --- src/renderers/dom/shared/DOMProperty.js | 1 - src/renderers/dom/shared/HTMLDOMPropertyConfig.js | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/dom/shared/DOMProperty.js b/src/renderers/dom/shared/DOMProperty.js index 5727ab0f4465a..efdd149663bfd 100644 --- a/src/renderers/dom/shared/DOMProperty.js +++ b/src/renderers/dom/shared/DOMProperty.js @@ -16,7 +16,6 @@ var invariant = require('fbjs/lib/invariant'); var RESERVED_PROPS = { children: true, dangerouslySetInnerHTML: true, - autoFocus: true, defaultValue: true, defaultChecked: true, innerHTML: true, diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index 3571ad955cbf5..45a3669fa6f86 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -26,6 +26,7 @@ var HTMLDOMPropertyConfig = { // name warnings. Properties: { allowFullScreen: HAS_BOOLEAN_VALUE, + autoFocus: HAS_STRING_BOOLEAN_VALUE, // specifies target context for links with `preload` type async: HAS_BOOLEAN_VALUE, // autoFocus is polyfilled/normalized by AutoFocusUtils From 536c1cf5a269067c5a6c84e6203e5249d57eae8a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 11 Oct 2017 16:22:47 +0100 Subject: [PATCH 2/4] Don't emit autoFocus attribute on the client --- src/renderers/dom/fiber/ReactDOMFiberComponent.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/renderers/dom/fiber/ReactDOMFiberComponent.js b/src/renderers/dom/fiber/ReactDOMFiberComponent.js index 2b75e96390b75..a98800329222b 100644 --- a/src/renderers/dom/fiber/ReactDOMFiberComponent.js +++ b/src/renderers/dom/fiber/ReactDOMFiberComponent.js @@ -54,6 +54,7 @@ var registrationNameModules = EventPluginRegistry.registrationNameModules; var DANGEROUSLY_SET_INNER_HTML = 'dangerouslySetInnerHTML'; var SUPPRESS_CONTENT_EDITABLE_WARNING = 'suppressContentEditableWarning'; var SUPPRESS_HYDRATION_WARNING = 'suppressHydrationWarning'; +var AUTOFOCUS = 'autoFocus'; var CHILDREN = 'children'; var STYLE = 'style'; var HTML = '__html'; @@ -286,6 +287,9 @@ function setInitialDOMProperties( propKey === SUPPRESS_HYDRATION_WARNING ) { // Noop + } else if (propKey === AUTOFOCUS) { + // We polyfill it separately on the client during commit. + // We blacklist it here rather than in the property list because we emit it in SSR. } else if (registrationNameModules.hasOwnProperty(propKey)) { if (nextProp != null) { if (__DEV__ && typeof nextProp !== 'function') { @@ -681,6 +685,8 @@ var ReactDOMFiberComponent = { propKey === SUPPRESS_HYDRATION_WARNING ) { // Noop + } else if (propKey === AUTOFOCUS) { + // Noop. It doesn't work on updates anyway. } else if (registrationNameModules.hasOwnProperty(propKey)) { // This is a special case. If any listener updates we need to ensure // that the "current" fiber pointer gets updated so we need a commit From 77b966626e6844f3140de8e10ae4e5ac28d73aec Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 11 Oct 2017 17:06:44 +0100 Subject: [PATCH 3/4] Test that hydration doesn't call focus --- .../__tests__/ReactServerRendering-test.js | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js index 30d93363b5cb2..766265b7181a3 100644 --- a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js @@ -351,6 +351,26 @@ describe('ReactDOMServer', () => { expect(numClicks).toEqual(2); }); + // We have a polyfill for autoFocus on the client, but we intentionally don't + // want it to call focus() when hydrating because this can mess up existing + // focus before the JS has loaded. + it('should emit autofocus on the server but not focus() when hydrating', () => { + var element = document.createElement('div'); + element.innerHTML = ReactDOMServer.renderToString( + , + ); + expect(element.firstChild.autofocus).toBe(true); + + // It should not be called on mount. + element.firstChild.focus = jest.fn(); + ReactDOM.hydrate(, element); + expect(element.firstChild.focus).not.toHaveBeenCalled(); + + // Or during an update. + ReactDOM.render(, element); + expect(element.firstChild.focus).not.toHaveBeenCalled(); + }); + it('should throw with silly args', () => { expect( ReactDOMServer.renderToString.bind(ReactDOMServer, {x: 123}), From c3b771a55e9ce3c334ce542981ec20e06326bad0 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 11 Oct 2017 17:34:20 +0100 Subject: [PATCH 4/4] Add autoFocus to SSR fixture --- fixtures/ssr/src/components/Page.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/fixtures/ssr/src/components/Page.js b/fixtures/ssr/src/components/Page.js index 806d1115c8207..778f29d56022e 100644 --- a/fixtures/ssr/src/components/Page.js +++ b/fixtures/ssr/src/components/Page.js @@ -2,6 +2,11 @@ import React, {Component} from 'react'; import './Page.css'; +const autofocusedInputs = [ + , + , +]; + export default class Page extends Component { state = {active: false}; handleClick = e => { @@ -18,9 +23,16 @@ export default class Page extends Component {

A random number: {Math.random()}

+

+ Autofocus on page load: {autofocusedInputs} +

{!this.state.active ? link : 'Thanks!'}

+ {this.state.active && +

+ Autofocus on update: {autofocusedInputs} +

} ); }