Skip to content

Fix boolean element attributes as per HTML5 spec #993

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

Closed
wants to merge 11 commits into from
2 changes: 1 addition & 1 deletion docs/docs/07-working-with-the-browser.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
4 changes: 0 additions & 4 deletions docs/docs/08-tooling-integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
2 changes: 1 addition & 1 deletion docs/docs/09.1-animation.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var TodoList = React.createClass({
render: function() {
var items = this.state.items.map(function(item, i) {
return (
<div key={i} onClick={this.handleRemove.bind(this, i)}>
<div key={item} onClick={this.handleRemove.bind(this, i)}>
{item}
</div>
);
Expand Down
1 change: 0 additions & 1 deletion docs/docs/10-examples.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
2 changes: 1 addition & 1 deletion docs/docs/ref-01-top-level-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
2 changes: 1 addition & 1 deletion docs/docs/ref-08-reconciliation.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ renderB: <div><span>second</span><span>first</span></div>
=> [replaceAttribute textContent 'second'], [insertNode <span>first</span>]
```

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(n<sup>2</sup>). 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(n<sup>2</sup>). 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

Expand Down
31 changes: 31 additions & 0 deletions docs/tips/16-references-to-components.md
Original file line number Diff line number Diff line change
@@ -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(<MyComponent />, 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 = <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.
48 changes: 41 additions & 7 deletions src/core/ReactComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,15 @@ 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 = {};

var NUMERIC_PROPERTY_REGEX = /^\d+$/;

/**
* Warn if the component doesn't have an explicit key assigned to it.
Expand All @@ -71,10 +75,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 + '.';
Expand All @@ -95,8 +99,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 (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)) {
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.
Expand All @@ -113,6 +143,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);
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/core/ReactPropTransferer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
/**
Expand Down
4 changes: 4 additions & 0 deletions src/dom/DOMPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ var DOMPropertyOperations = {
return '';
}
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)) {
Expand Down
4 changes: 2 additions & 2 deletions src/dom/__tests__/DOMPropertyOperations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ describe('DOMPropertyOperations', function() {
expect(DOMPropertyOperations.createMarkupForProperty(
'checked',
'simple'
)).toBe('checked="simple"');
)).toBe('checked');

expect(DOMPropertyOperations.createMarkupForProperty(
'checked',
true
)).toBe('checked="true"');
)).toBe('checked');

expect(DOMPropertyOperations.createMarkupForProperty(
'checked',
Expand Down
13 changes: 10 additions & 3 deletions src/dom/components/ReactDOMInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
52 changes: 52 additions & 0 deletions src/utils/__tests__/cloneWithProps-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,56 @@ describe('cloneWithProps', function() {
cloneWithProps(<Component />, {key: 'xyz'})
);
});

it('should transfer children', function() {
var Component = React.createClass({
render: function() {
expect(this.props.children).toBe('xyz');
return <div />;
}
});

ReactTestUtils.renderIntoDocument(
cloneWithProps(<Component />, {children: 'xyz'})
);
});

it('should shallow clone children', function() {
var Component = React.createClass({
render: function() {
expect(this.props.children).toBe('xyz');
return <div />;
}
});

ReactTestUtils.renderIntoDocument(
cloneWithProps(<Component>xyz</Component>, {})
);
});

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 <div />;
}
});

var Parent = React.createClass({
render: function() {
var clone =
cloneWithProps(this.props.children, {key: 'xyz', ref: 'xyz'});
return <div>{clone}</div>;
}
});

var Grandparent = React.createClass({
render: function() {
return <Parent><Component key="abc" ref="abc" /></Parent>;
}
});

ReactTestUtils.renderIntoDocument(<Grandparent />);
});
});
29 changes: 27 additions & 2 deletions src/utils/cloneWithProps.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
}

Expand Down