Skip to content

Commit

Permalink
Do not clone key and ref getters
Browse files Browse the repository at this point in the history
  • Loading branch information
ericmatthys committed Apr 8, 2016
1 parent 2b1bd1d commit ac48085
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 10 deletions.
38 changes: 28 additions & 10 deletions src/isomorphic/classic/element/ReactElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@ var RESERVED_PROPS = {

var specialPropKeyWarningShown, specialPropRefWarningShown;

function isValidConfigRefOrKey(config, name) {
if (__DEV__) {
return config.hasOwnProperty(name) &&
!Object.getOwnPropertyDescriptor(config, name).get;
}

return config[name] !== undefined;
}

function getConfigKey(config) {
return '' + config.key;
}

/**
* Factory method to create a new React element. This no longer adheres to
* the class pattern, so do not use new to call it. Also, no instanceof check
Expand Down Expand Up @@ -133,14 +146,16 @@ ReactElement.createElement = function(type, config, children) {
'React.createElement(...): Expected props argument to be a plain object. ' +
'Properties defined in its prototype chain will be ignored.'
);
ref = !config.hasOwnProperty('ref') ||
Object.getOwnPropertyDescriptor(config, 'ref').get ? null : config.ref;
key = !config.hasOwnProperty('key') ||
Object.getOwnPropertyDescriptor(config, 'key').get ? null : '' + config.key;
} else {
ref = config.ref === undefined ? null : config.ref;
key = config.key === undefined ? null : '' + config.key;
}

if (isValidConfigRefOrKey(config, 'ref')) {
ref = config.ref;
}

if (isValidConfigRefOrKey(config, 'key')) {
key = getConfigKey(config);
}

self = config.__self === undefined ? null : config.__self;
source = config.__source === undefined ? null : config.__source;
// Remaining properties are added to a new props object
Expand Down Expand Up @@ -284,14 +299,17 @@ ReactElement.cloneElement = function(element, config, children) {
'Properties defined in its prototype chain will be ignored.'
);
}
if (config.ref !== undefined) {

if (isValidConfigRefOrKey(config, 'ref')) {
// Silently steal the ref from the parent.
ref = config.ref;
owner = ReactCurrentOwner.current;
}
if (config.key !== undefined) {
key = '' + config.key;

if (isValidConfigRefOrKey(config, 'key')) {
key = getConfigKey(config);
}

// Remaining properties override existing props
var defaultProps;
if (element.type && element.type.defaultProps) {
Expand Down
80 changes: 80 additions & 0 deletions src/isomorphic/classic/element/__tests__/ReactElement-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,86 @@ describe('ReactElement', function() {
expect(element.props).toEqual(expectation);
});

it('should not extract key and ref getters from the config when creating an element', function() {
var props = {
foo: '56',
};

Object.defineProperty(props, 'key', {
get: function() {
return '12';
},
});

Object.defineProperty(props, 'ref', {
get: function() {
return '34';
},
});

var element = React.createFactory(ComponentClass)(props);
expect(element.type).toBe(ComponentClass);
expect(element.key).toBe(null);
expect(element.ref).toBe(null);
var expectation = {foo:'56'};
Object.freeze(expectation);
expect(element.props).toEqual(expectation);
});

it('should not extract key and ref getters from the config when cloning an element', function() {
var element = React.createFactory(ComponentClass)({
key: '12',
ref: '34',
foo: '56',
});

var props = {
foo: 'ef',
};

Object.defineProperty(props, 'key', {
get: function() {
return 'ab';
},
});

Object.defineProperty(props, 'ref', {
get: function() {
return 'cd';
},
});

var clone = React.cloneElement(element, props);
expect(clone.type).toBe(ComponentClass);
expect(clone.key).toBe('12');
expect(clone.ref).toBe('34');
var expectation = {foo:'ef'};
Object.freeze(expectation);
expect(clone.props).toEqual(expectation);
});

it('should allow null key and ref values when cloning an element', function() {
var element = React.createFactory(ComponentClass)({
key: '12',
ref: '34',
foo: '56',
});

var props = {
key: null,
ref: null,
foo: 'ef',
};

var clone = React.cloneElement(element, props);
expect(clone.type).toBe(ComponentClass);
expect(clone.key).toBe('null');
expect(clone.ref).toBe(null);
var expectation = {foo:'ef'};
Object.freeze(expectation);
expect(clone.props).toEqual(expectation);
});

it('coerces the key to a string', function() {
var element = React.createFactory(ComponentClass)({
key: 12,
Expand Down

0 comments on commit ac48085

Please sign in to comment.