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.

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

Inline DOM property configs #10805

Closed
wants to merge 13 commits into from
Closed

Inline DOM property configs #10805

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
cf2e246
Stop exposing DOMProperty to www
gaearon Sep 24, 2017
1fcd2f3
Remove DOM injection for attributeName
gaearon Sep 24, 2017
8f6dc26
Remove DOM injection for propertyName
gaearon Sep 24, 2017
cbca8df
Remove DOM injection for mutationMethod
gaearon Sep 24, 2017
13cc84b
Remove DOM injection for MUST_USE_PROPERTY flag
gaearon Sep 24, 2017
93f2aa4
Remove DOM injection for HAS_POSITIVE_NUMERIC_VALUE flag
gaearon Sep 24, 2017
570e844
Move shouldIgnoreValue into DOMProperty
gaearon Sep 24, 2017
2017452
Remove DOM injection for remaining HAS_*_VALUE flags
gaearon Sep 24, 2017
8b47546
Remove getPropertyInfo() branching in SSR validation
gaearon Sep 25, 2017
061fb15
Remove getPropertyInfo() branching in SSR markup generation
gaearon Sep 25, 2017
bf858ef
Remove getPropertyInfo() branching in client-side logic
gaearon Sep 25, 2017
82dcc93
Remove getPropertyInfo() and remaining property configs
gaearon Sep 25, 2017
dea8439
Add failing test for invalid attribute warning
Sep 25, 2017
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
18 changes: 0 additions & 18 deletions scripts/rollup/shims/facebook-www/DOMProperty.js
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions src/fb/ReactDOMFiberFBEntry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ Object.assign(
ReactFiberTreeReflection: require('ReactFiberTreeReflection'),
ReactDOMComponentTree: require('ReactDOMComponentTree'),
ReactInstanceMap: require('ReactInstanceMap'),
// This is used for ajaxify on www:
DOMProperty: require('DOMProperty'),
// These are dependencies of TapEventPlugin:
EventPluginUtils: require('EventPluginUtils'),
EventPropagators: require('EventPropagators'),
Expand Down
2 changes: 0 additions & 2 deletions src/renderers/dom/ReactDOMServerBrowserEntry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ var ReactDOMStringRenderer = require('ReactDOMStringRenderer');
var ReactVersion = require('ReactVersion');
var invariant = require('fbjs/lib/invariant');

require('ReactDOMInjection');

module.exports = {
renderToString: ReactDOMStringRenderer.renderToString,
renderToStaticMarkup: ReactDOMStringRenderer.renderToStaticMarkup,
Expand Down
2 changes: 0 additions & 2 deletions src/renderers/dom/ReactDOMServerNodeEntry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ var ReactDOMStringRenderer = require('ReactDOMStringRenderer');
var ReactDOMNodeStreamRenderer = require('ReactDOMNodeStreamRenderer');
var ReactVersion = require('ReactVersion');

require('ReactDOMInjection');

module.exports = {
renderToString: ReactDOMStringRenderer.renderToString,
renderToStaticMarkup: ReactDOMStringRenderer.renderToStaticMarkup,
Expand Down
1 change: 0 additions & 1 deletion src/renderers/dom/ReactDOMServerStackEntry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
var ReactServerRendering = require('ReactServerRendering');
var ReactVersion = require('ReactVersion');

require('ReactDOMInjection');
require('ReactDOMStackInjection');

var ReactDOMServerStack = {
Expand Down
1 change: 0 additions & 1 deletion src/renderers/dom/ReactDOMStackEntry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ if (__DEV__) {
var warning = require('fbjs/lib/warning');
}

require('ReactDOMInjection');
require('ReactDOMClientInjection');
require('ReactDOMStackInjection');

Expand Down
38 changes: 14 additions & 24 deletions src/renderers/dom/fiber/ReactDOMFiberComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -974,7 +974,6 @@ var ReactDOMFiberComponent = {
} else if (__DEV__) {
// Validate that the properties correspond to their expected values.
var serverValue;
var propertyInfo;
if (
propKey === SUPPRESS_CONTENT_EDITABLE_WARNING ||
// Controlled attributes are not validated
Expand Down Expand Up @@ -1014,32 +1013,23 @@ var ReactDOMFiberComponent = {
warnForPropDifference(propKey, serverValue, nextProp);
}
} else if (DOMProperty.shouldSetAttribute(propKey, nextProp)) {
if ((propertyInfo = DOMProperty.getPropertyInfo(propKey))) {
let ownNamespace = parentNamespace;
if (ownNamespace === HTML_NAMESPACE) {
ownNamespace = getIntrinsicNamespace(tag);
}
const attributeName = DOMProperty.getAttributeName(propKey);
if (ownNamespace === HTML_NAMESPACE) {
// $FlowFixMe - Should be inferred as not undefined.
extraAttributeNames.delete(propertyInfo.attributeName);
serverValue = DOMPropertyOperations.getValueForProperty(
domElement,
propKey,
nextProp,
);
extraAttributeNames.delete(attributeName.toLowerCase());
} else {
let ownNamespace = parentNamespace;
if (ownNamespace === HTML_NAMESPACE) {
ownNamespace = getIntrinsicNamespace(tag);
}
if (ownNamespace === HTML_NAMESPACE) {
// $FlowFixMe - Should be inferred as not undefined.
extraAttributeNames.delete(propKey.toLowerCase());
} else {
// $FlowFixMe - Should be inferred as not undefined.
extraAttributeNames.delete(propKey);
}
serverValue = DOMPropertyOperations.getValueForAttribute(
domElement,
propKey,
nextProp,
);
// $FlowFixMe - Should be inferred as not undefined.
extraAttributeNames.delete(attributeName);
}
serverValue = DOMPropertyOperations.getValueForProperty(
domElement,
propKey,
nextProp,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

If anything, I already appreciate the reduction in branching.


if (nextProp !== serverValue) {
warnForPropDifference(propKey, serverValue, nextProp);
Expand Down
1 change: 0 additions & 1 deletion src/renderers/dom/fiber/ReactDOMFiberEntry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ if (__DEV__) {
}

require('ReactDOMClientInjection');
require('ReactDOMInjection');
ReactControlledComponent.injection.injectFiberControlledHostComponent(
ReactDOMFiberComponent,
);
Expand Down
51 changes: 16 additions & 35 deletions src/renderers/dom/shared/DOMMarkupOperations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,6 @@ function isAttributeNameSafe(attributeName) {
return false;
}

// shouldIgnoreValue() is currently duplicated in DOMPropertyOperations.
// TODO: Find a better place for this.
function shouldIgnoreValue(propertyInfo, value) {
return (
value == null ||
(propertyInfo.hasBooleanValue && !value) ||
(propertyInfo.hasNumericValue && isNaN(value)) ||
(propertyInfo.hasPositiveNumericValue && value < 1) ||
(propertyInfo.hasOverloadedBooleanValue && value === false)
);
}

/**
* Operations for dealing with DOM properties.
*/
Expand Down Expand Up @@ -88,30 +76,23 @@ var DOMMarkupOperations = {
* @return {?string} Markup string, or null if the property was invalid.
*/
createMarkupForProperty: function(name, value) {
var propertyInfo = DOMProperty.getPropertyInfo(name);
if (propertyInfo) {
if (shouldIgnoreValue(propertyInfo, value)) {
return '';
}
var attributeName = propertyInfo.attributeName;
if (
propertyInfo.hasBooleanValue ||
(propertyInfo.hasOverloadedBooleanValue && value === true)
) {
return attributeName + '=""';
} else if (
typeof value !== 'boolean' ||
DOMProperty.shouldAttributeAcceptBooleanValue(name)
) {
return attributeName + '=' + quoteAttributeValueForBrowser(value);
}
} else if (DOMProperty.shouldSetAttribute(name, value)) {
if (value == null) {
return '';
}
return name + '=' + quoteAttributeValueForBrowser(value);
// TODO: unify these checks?
if (
!DOMProperty.shouldSetAttribute(name, value) ||
DOMProperty.shouldIgnoreValue(name, value)
) {
return null;
}
var attributeName = DOMProperty.getAttributeName(name);
var expectedType = DOMProperty.getExpectedValueType(name);
if (
expectedType === 'boolean' ||
(expectedType === 'overloadedBoolean' && value === true)
) {
return attributeName + '=""';
} else {
return attributeName + '=' + quoteAttributeValueForBrowser(value);
}
return null;
},

/**
Expand Down
Loading