Skip to content

Commit

Permalink
Do not attach listeners when not using createElement:true
Browse files Browse the repository at this point in the history
Also disable tests that require `createElement:true` when running in
`createElement:false` mode.
  • Loading branch information
nhunzaker committed Apr 4, 2017
1 parent a2c7814 commit 0aafd7a
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 98 deletions.
14 changes: 7 additions & 7 deletions src/renderers/dom/fiber/ReactDOMFiberComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ if (__DEV__) {
function getDocument(rootContainerElement) {
var isDocumentFragment = rootContainerElement.nodeType === DOC_FRAGMENT_TYPE;
var doc = isDocumentFragment
? rootContainerElement
: rootContainerElement.ownerDocument;
? rootContainerElement
: rootContainerElement.ownerDocument;

return doc;
}
Expand Down Expand Up @@ -238,7 +238,7 @@ function setInitialDOMProperties(
// Noop
} else if (registrationNameModules.hasOwnProperty(propKey)) {
if (nextProp) {
listenTo(propKey, doc, domElement)
listenTo(propKey, doc, domElement);
}
} else if (isCustomComponentTag) {
DOMPropertyOperations.setValueForAttribute(domElement, propKey, nextProp);
Expand Down Expand Up @@ -350,7 +350,7 @@ var ReactDOMFiberComponent = {
warning(
type === type.toLowerCase() || isCustomComponent(type, props),
'<%s /> is using uppercase HTML. Always use lowercase HTML tags ' +
'in React.',
'in React.',
type,
);
}
Expand Down Expand Up @@ -392,7 +392,7 @@ var ReactDOMFiberComponent = {
warning(
false,
'%s is using shady DOM. Using shady DOM with React can ' +
'cause things to break subtly.',
'cause things to break subtly.',
getCurrentFiberOwnerName() || 'A component',
);
didWarnShadyDOM = true;
Expand All @@ -418,7 +418,7 @@ var ReactDOMFiberComponent = {
props = ReactDOMFiberInput.getHostProps(domElement, rawProps);
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
listenTo('onChange', doc, domElement)
listenTo('onChange', doc, domElement);
break;
case 'option':
ReactDOMFiberOption.mountWrapper(domElement, rawProps);
Expand All @@ -429,7 +429,7 @@ var ReactDOMFiberComponent = {
props = ReactDOMFiberSelect.getHostProps(domElement, rawProps);
// For controlled components we always need to ensure we're listening
// to onChange. Even if there is no listener.
listenTo('onChange', doc, domElement)
listenTo('onChange', doc, domElement);
break;
case 'textarea':
ReactDOMFiberTextarea.mountWrapper(domElement, rawProps);
Expand Down
92 changes: 49 additions & 43 deletions src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -756,28 +756,31 @@ describe('ReactDOMComponent', () => {
};
});

it('should work error event on <source> element', () => {
var container = document.createElement('div');
var onError = jest.fn();
if (ReactDOMFeatureFlags.useCreateElement) {
// We do not attach DOM listeners when not using create element
it('should work error event on <source> element', () => {
var container = document.createElement('div');
var onError = jest.fn();

ReactDOM.render(
<video>
<source
src="http://example.org/video"
type="video/mp4"
onError={() => onError('onError called')}
/>
</video>,
container,
);
ReactDOM.render(
<video>
<source
src="http://example.org/video"
type="video/mp4"
onError={() => onError('onError called')}
/>
</video>,
container,
);

var errorEvent = document.createEvent('Event');
errorEvent.initEvent('error', false, false);
container.querySelector('source').dispatchEvent(errorEvent);
var errorEvent = document.createEvent('Event');
errorEvent.initEvent('error', false, false);
container.querySelector('source').dispatchEvent(errorEvent);

expect(onError).toHaveBeenCalledTimes(1);
expect(onError).toHaveBeenCalledWith('onError called');
});
expect(onError).toHaveBeenCalledTimes(1);
expect(onError).toHaveBeenCalledWith('onError called');
});
}

it('should not duplicate uppercased selfclosing tags', () => {
spyOn(console, 'error');
Expand Down Expand Up @@ -1055,35 +1058,38 @@ describe('ReactDOMComponent', () => {
}
});

it('should work load and error events on <image> element in SVG', () => {
var onError = jest.fn();
var onLoad = jest.fn();
if (ReactDOMFeatureFlags.useCreateElement) {
// We do not attach DOM listeners when not using create element
it('should work load and error events on <image> element in SVG', () => {
var onError = jest.fn();
var onLoad = jest.fn();

var container = document.createElement('div');
ReactDOM.render(
<svg>
<image
xlinkHref="http://example.org/image"
onError={() => onError('onError called')}
onLoad={() => onLoad('onLoad called')}
/>
</svg>,
container,
);
var container = document.createElement('div');
ReactDOM.render(
<svg>
<image
xlinkHref="http://example.org/image"
onError={() => onError('onError called')}
onLoad={() => onLoad('onLoad called')}
/>
</svg>,
container,
);

var loadEvent = new Event('load');
var errorEvent = new Event('error');
var image = container.querySelector('image');
var loadEvent = new Event('load');
var errorEvent = new Event('error');
var image = container.querySelector('image');

image.dispatchEvent(errorEvent);
image.dispatchEvent(loadEvent);
image.dispatchEvent(errorEvent);
image.dispatchEvent(loadEvent);

expect(onError).toHaveBeenCalledTimes(1);
expect(onError).toHaveBeenCalledWith('onError called');
expect(onError).toHaveBeenCalledTimes(1);
expect(onError).toHaveBeenCalledWith('onError called');

expect(onLoad).toHaveBeenCalledTimes(1);
expect(onLoad).toHaveBeenCalledWith('onLoad called');
});
expect(onLoad).toHaveBeenCalledTimes(1);
expect(onLoad).toHaveBeenCalledWith('onLoad called');
});
}
});

describe('updateComponent', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ var ReactDOM;
var ReactDOMComponentTree;
var ReactTestUtils;
var SelectEventPlugin;
var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');

describe('SelectEventPlugin', () => {
function extract(node, topLevelEvent) {
Expand Down Expand Up @@ -59,34 +60,37 @@ describe('SelectEventPlugin', () => {
expect(mouseup).toBe(null);
});

it('should extract if an `onSelect` listener is present', () => {
class WithSelect extends React.Component {
render() {
return <input type="text" onSelect={this.props.onSelect} />;
if (ReactDOMFeatureFlags.useCreateElement) {
// We do not attach DOM listeners when not using create element
it('should extract if an `onSelect` listener is present', () => {
class WithSelect extends React.Component {
render() {
return <input type="text" onSelect={this.props.onSelect} />;
}
}
}

var cb = jest.fn();
var cb = jest.fn();

var rendered = ReactTestUtils.renderIntoDocument(
<WithSelect onSelect={cb} />,
);
var node = ReactDOM.findDOMNode(rendered);
var rendered = ReactTestUtils.renderIntoDocument(
<WithSelect onSelect={cb} />,
);
var node = ReactDOM.findDOMNode(rendered);

node.selectionStart = 0;
node.selectionEnd = 0;
node.focus();
node.selectionStart = 0;
node.selectionEnd = 0;
node.focus();

var focus = extract(node, 'topFocus');
expect(focus).toBe(null);
var focus = extract(node, 'topFocus');
expect(focus).toBe(null);

var mousedown = extract(node, 'topMouseDown');
expect(mousedown).toBe(null);
var mousedown = extract(node, 'topMouseDown');
expect(mousedown).toBe(null);

var mouseup = extract(node, 'topMouseUp');
expect(mouseup).not.toBe(null);
expect(typeof mouseup).toBe('object');
expect(mouseup.type).toBe('select');
expect(mouseup.target).toBe(node);
});
var mouseup = extract(node, 'topMouseUp');
expect(mouseup).not.toBe(null);
expect(typeof mouseup).toBe('object');
expect(mouseup.type).toBe('select');
expect(mouseup.target).toBe(node);
});
}
});
18 changes: 11 additions & 7 deletions src/renderers/dom/shared/wrappers/__tests__/ReactDOMIframe-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,27 @@ describe('ReactDOMIframe', () => {
var React;
var ReactDOM;
var ReactTestUtils;
var ReactDOMFeatureFlags;

beforeEach(() => {
React = require('react');
ReactDOM = require('react-dom');
ReactTestUtils = require('ReactTestUtils');
ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
});

it('should trigger load events', () => {
var onLoadSpy = jasmine.createSpy();
var iframe = React.createElement('iframe', {onLoad: onLoadSpy});
iframe = ReactTestUtils.renderIntoDocument(iframe);
if (ReactDOMFeatureFlags.createElement) {
var onLoadSpy = jasmine.createSpy();
var iframe = React.createElement('iframe', {onLoad: onLoadSpy});
iframe = ReactTestUtils.renderIntoDocument(iframe);

var loadEvent = document.createEvent('Event');
loadEvent.initEvent('load', false, false);
var loadEvent = document.createEvent('Event');
loadEvent.initEvent('load', false, false);

ReactDOM.findDOMNode(iframe).dispatchEvent(loadEvent);
ReactDOM.findDOMNode(iframe).dispatchEvent(loadEvent);

expect(onLoadSpy).toHaveBeenCalled();
expect(onLoadSpy).toHaveBeenCalled();
}
});
});
18 changes: 0 additions & 18 deletions src/renderers/dom/stack/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,22 +144,6 @@ function getDocument(inst) {
return doc;
}

function ensureListeners() {
var inst = this;

var props = inst._currentElement.props;
var doc = getDocument(inst);
var node = getNode(inst);

for (var propKey in props) {
if (registrationNameModules.hasOwnProperty(propKey)) {
if (!!props[propKey]) {
listenTo(propKey, doc, node);
}
}
}
}

function inputPostMount() {
var inst = this;
// For controlled components we always need to ensure we're listening
Expand Down Expand Up @@ -606,8 +590,6 @@ ReactDOMComponent.Mixin = {
return ret;
}

transaction.getReactMountReady().enqueue(ensureListeners, this);

if (!this._hostParent) {
ret += ' ' + DOMPropertyOperations.createMarkupForRoot();
}
Expand Down

0 comments on commit 0aafd7a

Please sign in to comment.