Skip to content

Shortened generated "data-reactid" #934

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 20, 2014
+101 −73
Merged

Shortened generated "data-reactid" #934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/addons/transitions/__tests__/ReactTransitionKeySet-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ describe('ReactTransitionKeySet', function() {
var component = <div>{one}{two}</div>;
expect(ReactTransitionKeySet.getChildMapping(component.props.children))
.toEqual({
'{one}': one,
'{two}': two
'.$one': one,
'.$two': two
});
});

Expand All @@ -48,8 +48,8 @@ describe('ReactTransitionKeySet', function() {
var two = <div key="two" />;
var component = <div>{one}{two}</div>;
expect(ReactTransitionKeySet.getKeySet(component.props.children)).toEqual({
'{one}': true,
'{two}': true
'.$one': true,
'.$two': true
});
});

Expand Down
11 changes: 7 additions & 4 deletions src/core/ReactInstanceHandles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ var MAX_TREE_DEPTH = 100;
* @internal
*/
function getReactRootIDString(index) {
return SEPARATOR + 'r[' + index.toString(36) + ']';
return SEPARATOR + index.toString(36);
}

/**
Expand Down Expand Up @@ -241,7 +241,7 @@ var ReactInstanceHandles = {
* @internal
*/
createReactID: function(rootID, name) {
return rootID + SEPARATOR + name;
return rootID + name;
},

/**
Expand All @@ -253,8 +253,11 @@ var ReactInstanceHandles = {
* @internal
*/
getReactRootIDFromNodeID: function(id) {
var regexResult = /\.r\[[^\]]+\]/.exec(id);
return regexResult && regexResult[0];
if (id && id.charAt(0) === SEPARATOR && id.length > 1) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this old regex test was more necessary when we were using the id attribute; whether this function returns anything is used by ReactMount.registerContainer in places to test if a React component is mounted into a container. Shouldn't be a big problem, just wanted to call it out explicitly. Maybe best to make sure this function returns null if the ID doesn't start with a dot.

var index = id.indexOf(SEPARATOR, 1);
return index > -1 ? id.substr(0, index) : id;
}
return null;
},

/**
Expand Down
4 changes: 2 additions & 2 deletions src/core/ReactMultiChild.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ var ReactMultiChild = {
var child = children[name];
if (children.hasOwnProperty(name)) {
// Inlined for performance, see `ReactInstanceHandles.createReactID`.
var rootID = this._rootNodeID + '.' + name;
var rootID = this._rootNodeID + name;
var mountImage = child.mountComponent(
rootID,
transaction,
Expand Down Expand Up @@ -385,7 +385,7 @@ var ReactMultiChild = {
*/
_mountChildByNameAtIndex: function(child, name, index, transaction) {
// Inlined for performance, see `ReactInstanceHandles.createReactID`.
var rootID = this._rootNodeID + '.' + name;
var rootID = this._rootNodeID + name;
var mountImage = child.mountComponent(
rootID,
transaction,
Expand Down
32 changes: 17 additions & 15 deletions src/core/__tests__/ReactIdentity-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('ReactIdentity', function() {
ReactMount = require('ReactMount');
});

var idExp = /^\.r\[.+?\](.*)$/;
var idExp = /^\.[^.]+(.*)$/;
function checkId(child, expectedId) {
var actual = idExp.exec(ReactMount.getID(child));
var expected = idExp.exec(expectedId);
Expand All @@ -55,8 +55,8 @@ describe('ReactIdentity', function() {
React.renderComponent(instance, document.createElement('div'));
var node = instance.getDOMNode();
reactComponentExpect(instance).toBeDOMComponentWithChildCount(2);
checkId(node.childNodes[0], '.r[0].{first}[0]');
checkId(node.childNodes[1], '.r[0].{second}[0]');
checkId(node.childNodes[0], '.0.$first:0');
checkId(node.childNodes[1], '.0.$second:0');
});

it('should allow key property to express identity', function() {
Expand All @@ -71,10 +71,10 @@ describe('ReactIdentity', function() {
React.renderComponent(instance, document.createElement('div'));
var node = instance.getDOMNode();
reactComponentExpect(instance).toBeDOMComponentWithChildCount(4);
checkId(node.childNodes[0], '.r[0].{apple}');
checkId(node.childNodes[1], '.r[0].{banana}');
checkId(node.childNodes[2], '.r[0].{0}');
checkId(node.childNodes[3], '.r[0].{123}');
checkId(node.childNodes[0], '.0.$apple');
checkId(node.childNodes[1], '.0.$banana');
checkId(node.childNodes[2], '.0.$0');
checkId(node.childNodes[3], '.0.$123');
});

it('should use instance identity', function() {
Expand All @@ -96,12 +96,12 @@ describe('ReactIdentity', function() {
var node = instance.getDOMNode();
reactComponentExpect(instance).toBeDOMComponentWithChildCount(3);

checkId(node.childNodes[0], '.r[0].{wrap1}');
checkId(node.childNodes[0].firstChild, '.r[0].{wrap1}.{squirrel}');
checkId(node.childNodes[1], '.r[0].{wrap2}');
checkId(node.childNodes[1].firstChild, '.r[0].{wrap2}.{bunny}');
checkId(node.childNodes[2], '.r[0].[2]');
checkId(node.childNodes[2].firstChild, '.r[0].[2].{chipmunk}');
checkId(node.childNodes[0], '.0.$wrap1');
checkId(node.childNodes[0].firstChild, '.0.$wrap1.$squirrel');
checkId(node.childNodes[1], '.0.$wrap2');
checkId(node.childNodes[1].firstChild, '.0.$wrap2.$bunny');
checkId(node.childNodes[2], '.0.2');
checkId(node.childNodes[2].firstChild, '.0.2.$chipmunk');
});

function renderAComponentWithKeyIntoContainer(key, container) {
Expand All @@ -116,8 +116,10 @@ describe('ReactIdentity', function() {
expect(span1.getDOMNode()).not.toBe(null);
expect(span2.getDOMNode()).not.toBe(null);

checkId(span1.getDOMNode(), '.r[0].{' + key + '}');
checkId(span2.getDOMNode(), '.r[0].[1]{' + key + '}[0]');
key = key.replace(/=/g, '=0');

checkId(span1.getDOMNode(), '.0.$' + key);
checkId(span2.getDOMNode(), '.0.1:$' + key + ':0');
}

it('should allow any character as a key, in a detached parent', function() {
Expand Down
16 changes: 14 additions & 2 deletions src/core/__tests__/ReactInstanceHandles-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,25 @@ describe('ReactInstanceHandles', function() {

describe('getReactRootIDFromNodeID', function() {
it('should support strings', function() {
var test = '.r[s_0_1][0]..[1]';
var expected = '.r[s_0_1]';
var test = '.s_0_1.0..1';
var expected = '.s_0_1';
var actual = ReactInstanceHandles.getReactRootIDFromNodeID(test);
expect(actual).toEqual(expected);
});
});

describe('getReactRootIDFromNodeID', function() {
it('should return null for invalid IDs', function() {
var getReactRootIDFromNodeID = (
ReactInstanceHandles.getReactRootIDFromNodeID
);

expect(getReactRootIDFromNodeID(null)).toEqual(null);
expect(getReactRootIDFromNodeID('.')).toEqual(null);
expect(getReactRootIDFromNodeID('#')).toEqual(null);
});
});

describe('traverseTwoPhase', function() {
it("should not traverse when traversing outside DOM", function() {
var targetID = '';
Expand Down
4 changes: 3 additions & 1 deletion src/core/__tests__/ReactMultiChildReconcile-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ var stripEmptyValues = function(obj) {
* here. This relies on an implementation detail of the rendering system.
*/
var getOriginalKey = function(childName) {
return childName.slice('{'.length, childName.length - '}[0]'.length);
var match = childName.match(/^\.\$([^.]+)\:0$/);
expect(match).not.toBeNull();
return match[1];
};

/**
Expand Down
22 changes: 11 additions & 11 deletions src/utils/__tests__/ReactChildren-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ describe('ReactChildren', function() {
expect(mappedKeys.length).toBe(1);
expect(mappedChildren[mappedKeys[0]]).not.toBe(simpleKid);
expect(mappedChildren[mappedKeys[0]].props.children).toBe(simpleKid);
expect(mappedKeys[0]).toBe('{simple}');
expect(mappedKeys[0]).toBe('.$simple');
});

it('should invoke callback with the right context', function() {
Expand Down Expand Up @@ -162,7 +162,7 @@ describe('ReactChildren', function() {
expect(mappedKeys.length).toBe(5);
// Keys default to indices.
expect(mappedKeys).toEqual(
['{keyZero}', '[1]', '{keyTwo}', '[3]', '{keyFour}']
['.$keyZero', '.1', '.$keyTwo', '.3', '.$keyFour']
);

expect(callback).toHaveBeenCalledWith(zero, 0);
Expand Down Expand Up @@ -235,12 +235,12 @@ describe('ReactChildren', function() {
expect(mappedKeys.length).toBe(6);
// Keys default to indices.
expect(mappedKeys).toEqual([
'[0]{firstHalfKey}[0]{keyZero}',
'[0]{firstHalfKey}[0][1]',
'[0]{firstHalfKey}[0]{keyTwo}',
'[0]{secondHalfKey}[0][0]',
'[0]{secondHalfKey}[0]{keyFour}',
'[0]{keyFive}{keyFiveInner}'
'.0:$firstHalfKey:0:$keyZero',
'.0:$firstHalfKey:0:1',
'.0:$firstHalfKey:0:$keyTwo',
'.0:$secondHalfKey:0:0',
'.0:$secondHalfKey:0:$keyFour',
'.0:$keyFive:$keyFiveInner'
]);

expect(callback).toHaveBeenCalledWith(zero, 0);
Expand Down Expand Up @@ -282,15 +282,15 @@ describe('ReactChildren', function() {
</div>
);

var expectedForcedKeys = ['{keyZero}', '{keyOne}'];
var expectedForcedKeys = ['.$keyZero', '.$keyOne'];
var mappedChildrenForcedKeys =
ReactChildren.map(forcedKeys.props.children, mapFn);
var mappedForcedKeys = Object.keys(mappedChildrenForcedKeys);
expect(mappedForcedKeys).toEqual(expectedForcedKeys);

var expectedRemappedForcedKeys = [
'{{keyZero^C}{giraffe}',
'{{keyOne^C}[0]'
'.$=1$keyZero:$giraffe',
'.$=1$keyOne:0'
];
var remappedChildrenForcedKeys =
ReactChildren.map(mappedChildrenForcedKeys, mapFn);
Expand Down
14 changes: 7 additions & 7 deletions src/utils/__tests__/sliceChildren-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ describe('sliceChildren', function() {
];
var children = renderAndSlice(fullSet, 0);
expect(children).toEqual({
'{A}': fullSet[0],
'{B}': fullSet[1],
'{C}': fullSet[2]
'.$A': fullSet[0],
'.$B': fullSet[1],
'.$C': fullSet[2]
});
});

Expand All @@ -82,8 +82,8 @@ describe('sliceChildren', function() {
];
var children = renderAndSlice(fullSet, 1);
expect(children).toEqual({
'{B}': fullSet[1],
'{C}': fullSet[2]
'.$B': fullSet[1],
'.$C': fullSet[2]
});
});

Expand All @@ -96,7 +96,7 @@ describe('sliceChildren', function() {
];
var children = renderAndSlice(fullSet, 1, 2);
expect(children).toEqual({
'{B}': fullSet[1]
'.$B': fullSet[1]
});
});

Expand All @@ -112,7 +112,7 @@ describe('sliceChildren', function() {
.instance();

expect(rendered.props.children).toEqual({
'[1]': b
'.1': b
});
});

Expand Down
32 changes: 16 additions & 16 deletions src/utils/__tests__/traverseAllChildren-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
simpleKid,
'{simple}',
'.$simple',
0
);
expect(traverseContext.length).toEqual(1);
Expand All @@ -62,7 +62,7 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
simpleKid,
'[0]',
'.0',
0
);
expect(traverseContext.length).toEqual(1);
Expand All @@ -81,7 +81,7 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
simpleKid,
'[0]',
'.0',
0
);
expect(traverseContext.length).toEqual(1);
Expand Down Expand Up @@ -114,21 +114,21 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
zero,
'{keyZero}',
'.$keyZero',
0
);
expect(traverseFn).toHaveBeenCalledWith(traverseContext, one, '[1]', 1);
expect(traverseFn).toHaveBeenCalledWith(traverseContext, one, '.1', 1);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
two,
'{keyTwo}',
'.$keyTwo',
2
);
expect(traverseFn).toHaveBeenCalledWith(traverseContext, three, '[3]', 3);
expect(traverseFn).toHaveBeenCalledWith(traverseContext, three, '.3', 3);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
four,
'{keyFour}',
'.$keyFour',
4
);
});
Expand Down Expand Up @@ -171,38 +171,38 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
zero,
'[0]{firstHalfKey}[0]{keyZero}',
'.0:$firstHalfKey:0:$keyZero',
0
);

expect(traverseFn)
.toHaveBeenCalledWith(traverseContext, one, '[0]{firstHalfKey}[0][1]', 1);
.toHaveBeenCalledWith(traverseContext, one, '.0:$firstHalfKey:0:1', 1);

expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
two,
'[0]{firstHalfKey}[0]{keyTwo}',
'.0:$firstHalfKey:0:$keyTwo',
2
);

expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
three,
'[0]{secondHalfKey}[0][0]',
'.0:$secondHalfKey:0:0',
3
);

expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
four,
'[0]{secondHalfKey}[0]{keyFour}',
'.0:$secondHalfKey:0:$keyFour',
4
);

expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
five,
'[0]{keyFive}{keyFiveInner}',
'.0:$keyFive:$keyFiveInner',
5
);
});
Expand All @@ -228,13 +228,13 @@ describe('traverseAllChildren', function() {
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
zeroForceKey,
'{keyZero}',
'.$keyZero',
0
);
expect(traverseFn).toHaveBeenCalledWith(
traverseContext,
oneForceKey,
'{keyOne}',
'.$keyOne',
1
);
});
Expand Down
Loading