From ec17e37e90c944d73f32281a9a76a626b6446f38 Mon Sep 17 00:00:00 2001 From: jjt Date: Wed, 29 Jan 2014 14:20:48 -0800 Subject: [PATCH 01/11] Fix boolean element attributes as per HTML5 spec --- src/dom/DOMPropertyOperations.js | 3 +++ src/dom/__tests__/DOMPropertyOperations-test.js | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/dom/DOMPropertyOperations.js b/src/dom/DOMPropertyOperations.js index 595fe556d62b6..d2fcc13514909 100644 --- a/src/dom/DOMPropertyOperations.js +++ b/src/dom/DOMPropertyOperations.js @@ -94,6 +94,9 @@ var DOMPropertyOperations = { if (shouldIgnoreValue(name, value)) { return ''; } + if (value === true && DOMProperty.hasBooleanValue[name]) { + return escapeTextForBrowser(name); + } var attributeName = DOMProperty.getAttributeName[name]; return processAttributeNameAndPrefix(attributeName) + escapeTextForBrowser(value) + '"'; diff --git a/src/dom/__tests__/DOMPropertyOperations-test.js b/src/dom/__tests__/DOMPropertyOperations-test.js index 1c4b222b5ef7e..60a26a7591540 100644 --- a/src/dom/__tests__/DOMPropertyOperations-test.js +++ b/src/dom/__tests__/DOMPropertyOperations-test.js @@ -87,7 +87,7 @@ describe('DOMPropertyOperations', function() { expect(DOMPropertyOperations.createMarkupForProperty( 'checked', true - )).toBe('checked="true"'); + )).toBe('checked'); expect(DOMPropertyOperations.createMarkupForProperty( 'checked', From 6ec9c62fe7dce8e9607ae2eee4b68714c1fef1d7 Mon Sep 17 00:00:00 2001 From: petehunt Date: Tue, 28 Jan 2014 16:02:16 -0800 Subject: [PATCH 02/11] remove references to react-page --- docs/docs/08-tooling-integration.md | 4 ---- docs/docs/10-examples.md | 1 - 2 files changed, 5 deletions(-) diff --git a/docs/docs/08-tooling-integration.md b/docs/docs/08-tooling-integration.md index 83e1adc40bc91..b032e9ccce12d 100644 --- a/docs/docs/08-tooling-integration.md +++ b/docs/docs/08-tooling-integration.md @@ -55,7 +55,3 @@ The open-source community has built tools that integrate JSX with several build that support `*.tmLanguage`. * Linting provides accurate line numbers after compiling without sourcemaps. * Elements use standard scoping so linters can find usage of out-of-scope components. - -## React Page - -To get started on a new project, you can use [react-page](https://github.com/facebook/react-page/), a complete React project creator. It supports both server-side and client-side rendering, source transform and packaging JSX files using CommonJS modules, and instant reload. diff --git a/docs/docs/10-examples.md b/docs/docs/10-examples.md index abf2577d2f6fe..3d9995362f243 100644 --- a/docs/docs/10-examples.md +++ b/docs/docs/10-examples.md @@ -17,6 +17,5 @@ prev: class-name-manipulation.html * We've included [a step-by-step comment box tutorial](/react/docs/tutorial.html). * [The React starter kit](/react/downloads.html) includes several examples which you can [view online in our GitHub repository](https://github.com/facebook/react/tree/master/examples/). -* [React Page](https://github.com/facebook/react-page) is a simple React project creator to get you up-and-running quickly with React. It supports both server-side and client-side rendering, source transform and packaging JSX files using CommonJS modules, and instant reload. * [React one-hour email](https://github.com/petehunt/react-one-hour-email/commits/master) goes step-by-step from a static HTML mock to an interactive email reader (written in just one hour!) * [Rendr + React app template](https://github.com/petehunt/rendr-react-template/) demonstrates how to use React's server rendering capabilities. From 9649d67f19457df09af7f61d701decf38ec12ba4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20O=E2=80=99Shannessy?= Date: Wed, 29 Jan 2014 13:06:22 -0800 Subject: [PATCH 03/11] Fix animation example code key should never be index into an array or there are bugs. Especially in transitions. Fixes #853 --- docs/docs/09.1-animation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/09.1-animation.md b/docs/docs/09.1-animation.md index fdcc40ae74b27..c3d6c23f5d553 100644 --- a/docs/docs/09.1-animation.md +++ b/docs/docs/09.1-animation.md @@ -35,7 +35,7 @@ var TodoList = React.createClass({ render: function() { var items = this.state.items.map(function(item, i) { return ( -
+
{item}
); From ca66ca03cf4a5d12d190fc8b2825329a2b63b2f2 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 29 Jan 2014 12:50:06 -0800 Subject: [PATCH 04/11] Use querySelectorAll instead of getElementsByName in ReactDOMInput. We've been able to use `querySelectorAll` in all the browsers that we support for some time now, but we haven't been able to test code that uses it in the older version of `jsdom` that we were using, until recently. Besides the general goal of modernizing our code, the impetus for this specific change is that I'm trying to support testing without having to render nodes into an actual document. The `.getElementsByName` method is only defined on `document` and only works if the nodes you care about are contained by the document. On the other hand, `querySelectorAll` works on any DOM node, and allows a more precise selection of just the `` elements that have the appropriate name. IE8's implementation of `querySelectorAll` supports attribute-based selectors, which is all we need here. --- src/dom/components/ReactDOMInput.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/dom/components/ReactDOMInput.js b/src/dom/components/ReactDOMInput.js index 27148e483ba6b..b461f711d8489 100644 --- a/src/dom/components/ReactDOMInput.js +++ b/src/dom/components/ReactDOMInput.js @@ -130,17 +130,24 @@ var ReactDOMInput = ReactCompositeComponent.createClass({ var name = this.props.name; if (this.props.type === 'radio' && name != null) { var rootNode = this.getDOMNode(); + var queryRoot = rootNode; + + while (queryRoot.parentNode) { + queryRoot = queryRoot.parentNode; + } + // If `rootNode.form` was non-null, then we could try `form.elements`, // but that sometimes behaves strangely in IE8. We could also try using // `form.getElementsByName`, but that will only return direct children // and won't include inputs that use the HTML5 `form=` attribute. Since // the input might not even be in a form, let's just use the global - // `getElementsByName` to ensure we don't miss anything. - var group = document.getElementsByName(name); + // `querySelectorAll` to ensure we don't miss anything. + var group = queryRoot.querySelectorAll( + 'input[name=' + JSON.stringify('' + name) + '][type="radio"]'); + for (var i = 0, groupLen = group.length; i < groupLen; i++) { var otherNode = group[i]; if (otherNode === rootNode || - otherNode.nodeName !== 'INPUT' || otherNode.type !== 'radio' || otherNode.form !== rootNode.form) { continue; } From 3cf8b2694b274101ac50f1a022dd41db11c10945 Mon Sep 17 00:00:00 2001 From: Josh Duck Date: Wed, 29 Jan 2014 12:50:25 -0800 Subject: [PATCH 05/11] Warn for numeric object properties Numeric keys will be ordered sequentially in Chrome/Firefox. Warn the user if such keys are set on a child. --- src/core/ReactComponent.js | 46 ++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index 45b9e688a98f2..b9775231a907b 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -43,11 +43,13 @@ var ComponentLifeCycle = keyMirror({ }); /** - * Warn if there's no key explicitly set on dynamic arrays of children. - * This allows us to keep track of children between updates. + * Warn if there's no key explicitly set on dynamic arrays of children or + * object keys are not valid. This allows us to keep track of children between + * updates. */ -var ownerHasWarned = {}; +var ownerHasExplicitKeyWarning = {}; +var ownerHasPropertyWarning = {}; /** * Warn if the component doesn't have an explicit key assigned to it. @@ -71,10 +73,10 @@ function validateExplicitKey(component) { // Name of the component whose render method tried to pass children. var currentName = ReactCurrentOwner.current.constructor.displayName; - if (ownerHasWarned.hasOwnProperty(currentName)) { + if (ownerHasExplicitKeyWarning.hasOwnProperty(currentName)) { return; } - ownerHasWarned[currentName] = true; + ownerHasExplicitKeyWarning[currentName] = true; var message = 'Each child in an array should have a unique "key" prop. ' + 'Check the render method of ' + currentName + '.'; @@ -95,8 +97,34 @@ function validateExplicitKey(component) { } /** - * Ensure that every component either is passed in a static location or, if - * if it's passed in an array, has an explicit key property defined. + * Warn if the key is being defined as an object property but has an incorrect + * value. + * + * @internal + * @param {string} name Property name of the key. + * @param {ReactComponent} component Component that requires a key. + */ +function validatePropertyKey(name) { + if (!isNaN(Number(name))) { + // Name of the component whose render method tried to pass children. + var currentName = ReactCurrentOwner.current.constructor.displayName; + if (ownerHasPropertyWarning.hasOwnProperty(currentName)) { + return; + } + ownerHasPropertyWarning[currentName] = true; + + console.warn( + 'Child objects should have non-numeric keys so ordering is preserved. ' + + 'Check the render method of ' + currentName + '. ' + + 'See http://fb.me/react-warning-keys for more information.' + ); + } +} + +/** + * Ensure that every component either is passed in a static location, in an + * array with an explicit keys property defined, or in an object literal + * with valid key property. * * @internal * @param {*} component Statically passed child of any type. @@ -113,6 +141,10 @@ function validateChildKeys(component) { } else if (ReactComponent.isValidComponent(component)) { // This component was passed in a valid location. component.__keyValidated__ = true; + } else if (component && typeof component === 'object') { + for (var name in component) { + validatePropertyKey(name, component); + } } } From c7c62ee28f419af541ee6fe4b79f1c640964fb6d Mon Sep 17 00:00:00 2001 From: Eric Schoffstall Date: Wed, 29 Jan 2014 20:19:51 -0700 Subject: [PATCH 06/11] fix grammar mistake --- docs/docs/ref-08-reconciliation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/ref-08-reconciliation.md b/docs/docs/ref-08-reconciliation.md index 1b3fcab0f0e03..0678d42f65ae6 100644 --- a/docs/docs/ref-08-reconciliation.md +++ b/docs/docs/ref-08-reconciliation.md @@ -103,7 +103,7 @@ renderB:
secondfirst
=> [replaceAttribute textContent 'second'], [insertNode first] ``` -There are many algorithms that attempt to find the minimum sets of operations to transform a list of elements. [Levenshtein distance](http://en.wikipedia.org/wiki/Levenshtein_distance) can find the minimum using single element insertion, deletion and substitution in O(n2). Even if we were to use Levenshtein, this doesn't find when a node has moved into another position and algorithms to do that have a much worst complexity. +There are many algorithms that attempt to find the minimum sets of operations to transform a list of elements. [Levenshtein distance](http://en.wikipedia.org/wiki/Levenshtein_distance) can find the minimum using single element insertion, deletion and substitution in O(n2). Even if we were to use Levenshtein, this doesn't find when a node has moved into another position and algorithms to do that have much worse complexity. ### Keys From d749b191b7a577f7f58870f3878512dd639c5c3f Mon Sep 17 00:00:00 2001 From: Pete Hunt Date: Thu, 30 Jan 2014 14:50:34 -0800 Subject: [PATCH 07/11] Support `children` and `ref` for `cloneWithProps()` We're not handling these correctly. --- src/core/ReactPropTransferer.js | 2 + src/utils/__tests__/cloneWithProps-test.js | 52 ++++++++++++++++++++++ src/utils/cloneWithProps.js | 29 +++++++++++- 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/core/ReactPropTransferer.js b/src/core/ReactPropTransferer.js index ed721a3c9b59d..77d83da6a2336 100644 --- a/src/core/ReactPropTransferer.js +++ b/src/core/ReactPropTransferer.js @@ -42,6 +42,8 @@ function createTransferStrategy(mergeStrategy) { /** * Transfer strategies dictate how props are transferred by `transferPropsTo`. + * NOTE: if you add any more exceptions to this list you should be sure to + * update `cloneWithProps()` accordingly. */ var TransferStrategies = { /** diff --git a/src/utils/__tests__/cloneWithProps-test.js b/src/utils/__tests__/cloneWithProps-test.js index 774b61de1edc6..b5aba846aeb7c 100644 --- a/src/utils/__tests__/cloneWithProps-test.js +++ b/src/utils/__tests__/cloneWithProps-test.js @@ -126,4 +126,56 @@ describe('cloneWithProps', function() { cloneWithProps(, {key: 'xyz'}) ); }); + + it('should transfer children', function() { + var Component = React.createClass({ + render: function() { + expect(this.props.children).toBe('xyz'); + return
; + } + }); + + ReactTestUtils.renderIntoDocument( + cloneWithProps(, {children: 'xyz'}) + ); + }); + + it('should shallow clone children', function() { + var Component = React.createClass({ + render: function() { + expect(this.props.children).toBe('xyz'); + return
; + } + }); + + ReactTestUtils.renderIntoDocument( + cloneWithProps(xyz, {}) + ); + }); + + it('should support keys and refs', function() { + var Component = React.createClass({ + render: function() { + expect(this.props.key).toBe('xyz'); + expect(this.props.ref).toBe('xyz'); + return
; + } + }); + + var Parent = React.createClass({ + render: function() { + var clone = + cloneWithProps(this.props.children, {key: 'xyz', ref: 'xyz'}); + return
{clone}
; + } + }); + + var Grandparent = React.createClass({ + render: function() { + return ; + } + }); + + ReactTestUtils.renderIntoDocument(); + }); }); diff --git a/src/utils/cloneWithProps.js b/src/utils/cloneWithProps.js index 28c3b4540dacc..310dd08672d6a 100644 --- a/src/utils/cloneWithProps.js +++ b/src/utils/cloneWithProps.js @@ -21,6 +21,14 @@ var ReactPropTransferer = require('ReactPropTransferer'); +var keyMirror = require('keyMirror'); + +var SpecialPropsToTransfer = keyMirror({ + key: null, + children: null, + ref: null +}); + /** * Sometimes you want to change the props of a child passed to you. Usually * this is to add a CSS class. @@ -42,10 +50,27 @@ function cloneWithProps(child, props) { } var newProps = ReactPropTransferer.mergeProps(child.props, props); - // ReactPropTransferer does not transfer the `key` prop so do it manually. - if (props.key) { + + // ReactPropTransferer does not transfer the `key` prop so do it manually. Do + // not transfer it from the original component. + if (props.hasOwnProperty(SpecialPropsToTransfer.key)) { newProps.key = props.key; } + + // ReactPropTransferer does not transfer the `children` prop. Transfer it + // from `props` if it exists, otherwise use `child.props.children` if it is + // provided. + if (props.hasOwnProperty(SpecialPropsToTransfer.children)) { + newProps.children = props.children; + } else if (child.props.hasOwnProperty(SpecialPropsToTransfer.children)) { + newProps.children = child.props.children; + } + + // ReactPropTransferer does not transfer `ref` so do it manually. + if (props.hasOwnProperty(SpecialPropsToTransfer.ref)) { + newProps.ref = props.ref; + } + return child.constructor.ConvenienceConstructor(newProps); } From 6ca23ae6492e098e63ec1f765723ba386c5522c7 Mon Sep 17 00:00:00 2001 From: Josh Duck Date: Thu, 30 Jan 2014 14:51:51 -0800 Subject: [PATCH 08/11] Fix warning for numeric properties Number('.1') === 0.1, and react uses dot-prefixed keys for children. Whoops. Nuke the non-numeric requirement, and just check a regex. This seems performant enough in micro-benchmarks: http://jsperf.com/numericlike --- src/core/ReactComponent.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/ReactComponent.js b/src/core/ReactComponent.js index b9775231a907b..c3e6df3d7c096 100644 --- a/src/core/ReactComponent.js +++ b/src/core/ReactComponent.js @@ -51,6 +51,8 @@ var ComponentLifeCycle = keyMirror({ var ownerHasExplicitKeyWarning = {}; var ownerHasPropertyWarning = {}; +var NUMERIC_PROPERTY_REGEX = /^\d+$/; + /** * Warn if the component doesn't have an explicit key assigned to it. * This component is in an array. The array could grow and shrink or be @@ -105,7 +107,7 @@ function validateExplicitKey(component) { * @param {ReactComponent} component Component that requires a key. */ function validatePropertyKey(name) { - if (!isNaN(Number(name))) { + if (NUMERIC_PROPERTY_REGEX.test(name)) { // Name of the component whose render method tried to pass children. var currentName = ReactCurrentOwner.current.constructor.displayName; if (ownerHasPropertyWarning.hasOwnProperty(currentName)) { From 815ac01798f04d8a41a587cb0295b046c57164a2 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Thu, 30 Jan 2014 17:08:29 -0800 Subject: [PATCH 09/11] Add documentation about React.renderComponent Recently learned that components passed into `React.renderComponent` may not be the ones actually mounted. Also learned that it returns the mounted component. Added some documentation describing this. --- docs/docs/ref-01-top-level-api.md | 2 +- docs/tips/16-references-to-components.md | 31 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 docs/tips/16-references-to-components.md diff --git a/docs/docs/ref-01-top-level-api.md b/docs/docs/ref-01-top-level-api.md index 4d64d00d8ab2c..48f764170f645 100644 --- a/docs/docs/ref-01-top-level-api.md +++ b/docs/docs/ref-01-top-level-api.md @@ -46,7 +46,7 @@ ReactComponent renderComponent( ) ``` -Render a React component into the DOM in the supplied `container`. +Render a React component into the DOM in the supplied `container` and return a reference to the component. If the React component was previously rendered into `container`, this will perform an update on it and only mutate the DOM as necessary to reflect the latest React component. diff --git a/docs/tips/16-references-to-components.md b/docs/tips/16-references-to-components.md new file mode 100644 index 0000000000000..38572276e61d7 --- /dev/null +++ b/docs/tips/16-references-to-components.md @@ -0,0 +1,31 @@ +--- +id: references-to-components +title: References to Components +layout: tips +permalink: references-to-components.html +prev: expose-component-functions.html +--- + +If you're using React components in a larger non-React application or transitioning your code to React, you may need to keep references to components. `React.renderComponent` returns a reference to the mounted component: + +```js +/** @jsx React.DOM */ + +var myComponent = React.renderComponent(, myContainer); +``` + +If you pass a variable to 'React.renderComponent`, it's not guaranteed that the component passed in will be the one that's mounted. In cases where you construct a component before mounting it, be sure to reassign your variable: + +```js +/** @jsx React.DOM */ + +var myComponent = ; + +// Some code here... + +myComponent = React.renderComponent(myComponent, myContainer); +``` + +> Note: +> +> This should only ever be used at the top level. Inside components, let your `props` and `state` handle communication with child components, and only reference components via `ref`s. From 157b87e9d5a0de76984676362a5a2c3613089cb7 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Fri, 31 Jan 2014 10:57:06 -0800 Subject: [PATCH 10/11] s/Mock DOM/Virtual DOM/ Let's be consistent with the naming --- docs/docs/07-working-with-the-browser.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/07-working-with-the-browser.md b/docs/docs/07-working-with-the-browser.md index c654f9a965dea..0ea557d480bb8 100644 --- a/docs/docs/07-working-with-the-browser.md +++ b/docs/docs/07-working-with-the-browser.md @@ -10,7 +10,7 @@ next: more-about-refs.html React provides powerful abstractions that free you from touching the DOM directly in most cases, but sometimes you simply need to access the underlying API, perhaps to work with a third-party library or existing code. -## The Mock DOM +## The Virtual DOM React is so fast because it never talks to the DOM directly. React maintains a fast in-memory representation of the DOM. `render()` methods return a *description* of the DOM, and React can diff this description with the in-memory representation to compute the fastest way to update the browser. From 16b6ddc7a07d402708a44c88e17a8499f5bd1e0f Mon Sep 17 00:00:00 2001 From: jjt Date: Fri, 31 Jan 2014 14:07:29 -0800 Subject: [PATCH 11/11] Boolean attributes use truthy value instead of strict true --- src/dom/DOMPropertyOperations.js | 7 ++++--- src/dom/__tests__/DOMPropertyOperations-test.js | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/dom/DOMPropertyOperations.js b/src/dom/DOMPropertyOperations.js index d2fcc13514909..b3b83d852cfcd 100644 --- a/src/dom/DOMPropertyOperations.js +++ b/src/dom/DOMPropertyOperations.js @@ -94,10 +94,11 @@ var DOMPropertyOperations = { if (shouldIgnoreValue(name, value)) { return ''; } - if (value === true && DOMProperty.hasBooleanValue[name]) { - return escapeTextForBrowser(name); - } var attributeName = DOMProperty.getAttributeName[name]; + console.log(attributeName); + if (value && DOMProperty.hasBooleanValue[name]) { + return escapeTextForBrowser(attributeName); + } return processAttributeNameAndPrefix(attributeName) + escapeTextForBrowser(value) + '"'; } else if (DOMProperty.isCustomAttribute(name)) { diff --git a/src/dom/__tests__/DOMPropertyOperations-test.js b/src/dom/__tests__/DOMPropertyOperations-test.js index 60a26a7591540..a754afa91c944 100644 --- a/src/dom/__tests__/DOMPropertyOperations-test.js +++ b/src/dom/__tests__/DOMPropertyOperations-test.js @@ -82,7 +82,7 @@ describe('DOMPropertyOperations', function() { expect(DOMPropertyOperations.createMarkupForProperty( 'checked', 'simple' - )).toBe('checked="simple"'); + )).toBe('checked'); expect(DOMPropertyOperations.createMarkupForProperty( 'checked',