Skip to content
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

Friendly key warnings #6500

Merged
merged 1 commit into from
Apr 16, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion src/renderers/shared/reconciler/ReactChildReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
var ReactReconciler = require('ReactReconciler');

var instantiateReactComponent = require('instantiateReactComponent');
var KeyEscapeUtils = require('KeyEscapeUtils');
var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
var traverseAllChildren = require('traverseAllChildren');
var warning = require('warning');
Expand All @@ -27,7 +28,7 @@ function instantiateChild(childInstances, child, name) {
'flattenChildren(...): Encountered two children with the same key, ' +
'`%s`. Child keys must be unique; when two children share a key, only ' +
'the first child will be used.',
name
KeyEscapeUtils.unescape(name)
);
}
if (child != null && keyUnique) {
Expand Down
64 changes: 64 additions & 0 deletions src/shared/utils/KeyEscapeUtils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/**
* Copyright 2013-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule KeyEscapeUtils
*/

'use strict';

/**
* Escape and wrap key so it is safe to use as a reactid
*
* @param {*} key to be escaped.
* @return {string} the escaped key.
*/
function escape(key) {
var escapeRegex = /[=:]/g;
var escaperLookup = {
'=': '=0',
':': '=2',
};
var escapedString = ('' + key).replace(
escapeRegex,
function(match) {
return escaperLookup[match];
}
);

return '$' + escapedString;
}

/**
* Unescape and unwrap key for human-readable display
*
* @param {string} key to unescape.
* @return {string} the unescaped key.
*/
function unescape(key) {
var unescapeRegex = /(=0|=2)/g;
var unescaperLookup = {
'=0': '=',
'=2': ':',
};
var keySubstring = (key[0] === '.' && key[1] === '$')
? key.substring(2) : key.substring(1);

return ('' + keySubstring).replace(
unescapeRegex,
function(match) {
return unescaperLookup[match];
}
);
}

var KeyEscapeUtils = {
escape: escape,
unescape: unescape,
};

module.exports = KeyEscapeUtils;
38 changes: 38 additions & 0 deletions src/shared/utils/__tests__/KeyEscapeUtils-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* Copyright 2013-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @emails react-core
*/

'use strict';

var KeyEscapeUtils;

describe('KeyEscapeUtils', () => {
beforeEach(() => {
jest.resetModuleRegistry();

KeyEscapeUtils = require('KeyEscapeUtils');
});

describe('escape', () => {
it('should properly escape and wrap user defined keys', () => {
expect(KeyEscapeUtils.escape('1')).toBe('$1');
expect(KeyEscapeUtils.escape('1=::=2')).toBe('$1=0=2=2=02');
});
});

describe('unescape', () => {
it('should properly unescape and unwrap user defined keys', () => {
expect(KeyEscapeUtils.unescape('.1')).toBe('1');
expect(KeyEscapeUtils.unescape('$1')).toBe('1');
expect(KeyEscapeUtils.unescape('.$1')).toBe('1');
expect(KeyEscapeUtils.unescape('$1=0=2=2=02')).toBe('1=::=2');
});
});
});
3 changes: 2 additions & 1 deletion src/shared/utils/flattenChildren.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

'use strict';

var KeyEscapeUtils = require('KeyEscapeUtils');
var traverseAllChildren = require('traverseAllChildren');
var warning = require('warning');

Expand All @@ -29,7 +30,7 @@ function flattenSingleChildIntoContext(traverseContext, child, name) {
'flattenChildren(...): Encountered two children with the same key, ' +
'`%s`. Child keys must be unique; when two children share a key, only ' +
'the first child will be used.',
name
KeyEscapeUtils.unescape(name)
);
}
if (keyUnique && child != null) {
Expand Down
40 changes: 3 additions & 37 deletions src/shared/utils/traverseAllChildren.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var ReactElement = require('ReactElement');

var getIteratorFn = require('getIteratorFn');
var invariant = require('invariant');
var KeyEscapeUtils = require('KeyEscapeUtils');
var warning = require('warning');

var SEPARATOR = '.';
Expand All @@ -26,19 +27,8 @@ var SUBSEPARATOR = ':';
* pattern.
*/

var userProvidedKeyEscaperLookup = {
'=': '=0',
':': '=2',
};

var userProvidedKeyEscapeRegex = /[=:]/g;

var didWarnAboutMaps = false;

function userProvidedKeyEscaper(match) {
return userProvidedKeyEscaperLookup[match];
}

/**
* Generate a key string that identifies a component within a set.
*
Expand All @@ -51,36 +41,12 @@ function getComponentKey(component, index) {
// that we don't block potential future ES APIs.
if (component && typeof component === 'object' && component.key != null) {
// Explicit key
return wrapUserProvidedKey(component.key);
return KeyEscapeUtils.escape(component.key);
}
// Implicit key determined by the index in the set
return index.toString(36);
}

/**
* Escape a component key so that it is safe to use in a reactid.
*
* @param {*} text Component key to be escaped.
* @return {string} An escaped string.
*/
function escapeUserProvidedKey(text) {
return ('' + text).replace(
userProvidedKeyEscapeRegex,
userProvidedKeyEscaper
);
}

/**
* Wrap a `key` value explicitly provided by the user to distinguish it from
* implicitly-generated keys generated by a component's index in its parent.
*
* @param {string} key Value of a user-provided `key` attribute
* @return {string}
*/
function wrapUserProvidedKey(key) {
return '$' + escapeUserProvidedKey(key);
}

/**
* @param {?*} children Children tree container.
* @param {!string} nameSoFar Name of the key path so far.
Expand Down Expand Up @@ -166,7 +132,7 @@ function traverseAllChildrenImpl(
child = entry[1];
nextName = (
nextNamePrefix +
wrapUserProvidedKey(entry[0]) + SUBSEPARATOR +
KeyEscapeUtils.escape(entry[0]) + SUBSEPARATOR +
getComponentKey(child, 0)
);
subtreeCount += traverseAllChildrenImpl(
Expand Down