Skip to content

Migrating to const/let and Object Spread in more places #11535

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
gaearon merged 2 commits into from
Nov 19, 2017
Merged

Migrating to const/let and Object Spread in more places #11535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8436555
Use const/let in more places (#11467)
raphamorim Nov 12, 2017
d4a58d9
Replace Object.assign by Object Spread
raphamorim Nov 12, 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
7 changes: 3 additions & 4 deletions packages/react-dom/src/client/ReactDOMFiberOption.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import React from 'react';
import warning from 'fbjs/lib/warning';

function flattenChildren(children) {
var content = '';
let content = '';

// Flatten children and warn if they aren't strings or numbers;
// invalid types are ignored.
Expand Down Expand Up @@ -52,9 +52,8 @@ export function postMountWrapper(element: Element, props: Object) {
}

export function getHostProps(element: Element, props: Object) {
var hostProps = Object.assign({children: undefined}, props);

var content = flattenChildren(props.children);
const hostProps = {children: undefined, ...props};
const content = flattenChildren(props.children);

if (content) {
hostProps.children = content;
Expand Down
32 changes: 16 additions & 16 deletions packages/react-dom/src/client/ReactDOMFiberTextarea.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import ReactDebugCurrentFiber from 'react-reconciler/src/ReactDebugCurrentFiber'

import ReactControlledValuePropTypes from '../shared/ReactControlledValuePropTypes';

var {getCurrentFiberStackAddendum} = ReactDebugCurrentFiber;
var didWarnValDefaultVal = false;
const {getCurrentFiberStackAddendum} = ReactDebugCurrentFiber;
let didWarnValDefaultVal = false;

type TextAreaWithWrapperState = HTMLTextAreaElement & {
_wrapperState: {
Expand All @@ -40,7 +40,7 @@ type TextAreaWithWrapperState = HTMLTextAreaElement & {
*/

export function getHostProps(element: Element, props: Object) {
var node = ((element: any): TextAreaWithWrapperState);
const node = ((element: any): TextAreaWithWrapperState);
invariant(
props.dangerouslySetInnerHTML == null,
'`dangerouslySetInnerHTML` does not make sense on <textarea>.',
Expand All @@ -52,17 +52,18 @@ export function getHostProps(element: Element, props: Object) {
// completely solve this IE9 bug), but Sebastian+Sophie seemed to like this
// solution. The value can be a boolean or object so that's why it's forced
// to be a string.
var hostProps = Object.assign({}, props, {
const hostProps = {
...props,
value: undefined,
defaultValue: undefined,
children: '' + node._wrapperState.initialValue,
});
};

return hostProps;
}

export function initWrapperState(element: Element, props: Object) {
var node = ((element: any): TextAreaWithWrapperState);
const node = ((element: any): TextAreaWithWrapperState);
if (__DEV__) {
ReactControlledValuePropTypes.checkPropTypes(
'textarea',
Expand All @@ -86,14 +87,13 @@ export function initWrapperState(element: Element, props: Object) {
}
}

var value = props.value;
var initialValue = value;
let initialValue = props.value;
Copy link
Copy Markdown
Contributor

@clemmy clemmy Nov 13, 2017

Choose a reason for hiding this comment

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

Hmm, wonder why it was written the other way initially. 😛

Copy link
Copy Markdown
Contributor Author

@raphamorim raphamorim Nov 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

te-hehe

Do you recommend putting it back?
I think it does not make sense to keep this line. Since it is the same reference.
Tradeoff: I guess it was written for better readability.

What do you think @clemmy

Copy link
Copy Markdown
Contributor

@clemmy clemmy Nov 13, 2017

Choose a reason for hiding this comment

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

I think this is probably fine.


// Only bother fetching default value if we're going to use it
if (value == null) {
var defaultValue = props.defaultValue;
if (initialValue == null) {
let defaultValue = props.defaultValue;
// TODO (yungsters): Remove support for children content in <textarea>.
var children = props.children;
let children = props.children;
if (children != null) {
if (__DEV__) {
warning(
Expand Down Expand Up @@ -128,12 +128,12 @@ export function initWrapperState(element: Element, props: Object) {
}

export function updateWrapper(element: Element, props: Object) {
var node = ((element: any): TextAreaWithWrapperState);
var value = props.value;
const node = ((element: any): TextAreaWithWrapperState);
const value = props.value;
if (value != null) {
// Cast `value` to a string to ensure the value is set correctly. While
// browsers typically do this as necessary, jsdom doesn't.
var newValue = '' + value;
const newValue = '' + value;

// To avoid side effects (such as losing text selection), only set value if changed
if (newValue !== node.value) {
Expand All @@ -149,10 +149,10 @@ export function updateWrapper(element: Element, props: Object) {
}

export function postMountWrapper(element: Element, props: Object) {
var node = ((element: any): TextAreaWithWrapperState);
const node = ((element: any): TextAreaWithWrapperState);
// This is in postMount because we need access to the DOM node, which is not
// available until after the component has mounted.
var textContent = node.textContent;
const textContent = node.textContent;

// Only set node.value if textContent is equal to the expected
// initial value. In IE10/IE11 there is a bug where the placeholder attribute
Expand Down
23 changes: 10 additions & 13 deletions packages/react-dom/src/client/ReactDOMSelection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,13 @@ import {TEXT_NODE} from '../shared/HTMLNodeType';
* @return {?object}
*/
export function getOffsets(outerNode) {
var selection = window.getSelection && window.getSelection();
const selection = window.getSelection && window.getSelection();

if (!selection || selection.rangeCount === 0) {
return null;
}

var anchorNode = selection.anchorNode;
var anchorOffset = selection.anchorOffset;
var focusNode = selection.focusNode;
var focusOffset = selection.focusOffset;
const {anchorNode, anchorOffset, focusNode, focusOffset} = selection;

// In Firefox, anchorNode and focusNode can be "anonymous divs", e.g. the
// up/down buttons on an <input type="number">. Anonymous divs do not seem to
Expand Down Expand Up @@ -157,21 +154,21 @@ export function setOffsets(node, offsets) {
return;
}

var selection = window.getSelection();
var length = node[getTextContentAccessor()].length;
var start = Math.min(offsets.start, length);
var end = offsets.end === undefined ? start : Math.min(offsets.end, length);
const selection = window.getSelection();
const length = node[getTextContentAccessor()].length;
let start = Math.min(offsets.start, length);
let end = offsets.end === undefined ? start : Math.min(offsets.end, length);

// IE 11 uses modern selection, but doesn't support the extend method.
// Flip backward selections, so we can set with a single range.
if (!selection.extend && start > end) {
var temp = end;
let temp = end;
end = start;
start = temp;
}

var startMarker = getNodeForCharacterOffset(node, start);
var endMarker = getNodeForCharacterOffset(node, end);
const startMarker = getNodeForCharacterOffset(node, start);
const endMarker = getNodeForCharacterOffset(node, end);

if (startMarker && endMarker) {
if (
Expand All @@ -183,7 +180,7 @@ export function setOffsets(node, offsets) {
) {
return;
}
var range = document.createRange();
const range = document.createRange();
range.setStart(startMarker.node, startMarker.offset);
selection.removeAllRanges();

Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/client/setTextContent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {TEXT_NODE} from '../shared/HTMLNodeType';
* @param {string} text
* @internal
*/
var setTextContent = function(node, text) {
let setTextContent = function(node, text) {
if (text) {
let firstChild = node.firstChild;

Expand Down
24 changes: 12 additions & 12 deletions packages/react-dom/src/client/validateDOMNesting.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import warning from 'fbjs/lib/warning';
import ReactDebugCurrentFiber from 'react-reconciler/src/ReactDebugCurrentFiber';

const {getCurrentFiberStackAddendum} = ReactDebugCurrentFiber;
var validateDOMNesting = emptyFunction;
let validateDOMNesting = emptyFunction;

if (__DEV__) {
// This validation code was written based on the HTML5 parsing spec:
Expand All @@ -26,7 +26,7 @@ if (__DEV__) {
// first, causing a confusing mess.

// https://html.spec.whatwg.org/multipage/syntax.html#special
var specialTags = [
const specialTags = [
'address',
'applet',
'area',
Expand Down Expand Up @@ -113,7 +113,7 @@ if (__DEV__) {
];

// https://html.spec.whatwg.org/multipage/syntax.html#has-an-element-in-scope
var inScopeTags = [
const inScopeTags = [
'applet',
'caption',
'html',
Expand All @@ -133,10 +133,10 @@ if (__DEV__) {
];

// https://html.spec.whatwg.org/multipage/syntax.html#has-an-element-in-button-scope
var buttonScopeTags = inScopeTags.concat(['button']);
const buttonScopeTags = inScopeTags.concat(['button']);

// https://html.spec.whatwg.org/multipage/syntax.html#generate-implied-end-tags
var impliedEndTags = [
const impliedEndTags = [
'dd',
'dt',
'li',
Expand All @@ -147,7 +147,7 @@ if (__DEV__) {
'rt',
];

var emptyAncestorInfo = {
const emptyAncestorInfo = {
current: null,

formTag: null,
Expand All @@ -160,9 +160,9 @@ if (__DEV__) {
dlItemTagAutoclosing: null,
};

var updatedAncestorInfo = function(oldInfo, tag, instance) {
var ancestorInfo = Object.assign({}, oldInfo || emptyAncestorInfo);
var info = {tag: tag, instance: instance};
const updatedAncestorInfo = function(oldInfo, tag, instance) {
let ancestorInfo = {...(oldInfo || emptyAncestorInfo)};
let info = {tag: tag, instance: instance};

if (inScopeTags.indexOf(tag) !== -1) {
ancestorInfo.aTagInScope = null;
Expand Down Expand Up @@ -215,7 +215,7 @@ if (__DEV__) {
/**
* Returns whether
*/
var isTagValidWithParent = function(tag, parentTag) {
const isTagValidWithParent = function(tag, parentTag) {
// First, let's check if we're in an unusual parsing mode...
switch (parentTag) {
// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inselect
Expand Down Expand Up @@ -337,7 +337,7 @@ if (__DEV__) {
/**
* Returns whether
*/
var findInvalidAncestorForTag = function(tag, ancestorInfo) {
const findInvalidAncestorForTag = function(tag, ancestorInfo) {
switch (tag) {
case 'address':
case 'article':
Expand Down Expand Up @@ -401,7 +401,7 @@ if (__DEV__) {
return null;
};

var didWarn = {};
const didWarn = {};

validateDOMNesting = function(childTag, childText, ancestorInfo) {
ancestorInfo = ancestorInfo || emptyAncestorInfo;
Expand Down