Skip to content

Commit

Permalink
Avoid meta programming to initialize functions in module scope (#26388)
Browse files Browse the repository at this point in the history
I'm trying to get rid of all meta programming in the module scope so
that closure can do a better job figuring out cyclic dependencies and
ability to reorder.

This is converting a lot of the patterns that assign functions
conditionally to using function declarations instead.

```
let fn;
if (__DEV__) {
  fn = function() {
    ...
  };
}
```
->
```
function fn() {
  if (__DEV__) {
    ...
  }
}
```
  • Loading branch information
sebmarkbage authored Mar 15, 2023
1 parent 21aee59 commit d310d65
Show file tree
Hide file tree
Showing 24 changed files with 628 additions and 652 deletions.
2 changes: 1 addition & 1 deletion packages/dom-event-testing-library/domEnvironment.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* Change environment support for PointerEvent.
*/

const emptyFunction = function () {};
function emptyFunction() {}

export function hasPointerEvent() {
return global != null && global.PointerEvent != null;
Expand Down
66 changes: 33 additions & 33 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,8 @@ const HTML = '__html';
let warnedUnknownTags: {
[key: string]: boolean,
};

let validatePropertiesInDevelopment;
let warnForPropDifference;
let warnForExtraAttributes;
let warnForInvalidEventListener;
let canDiffStyleForHydrationWarning;

let normalizeHTML;

if (__DEV__) {
warnedUnknownTags = {
// There are working polyfills for <dialog>. Let people use it.
Expand All @@ -115,15 +108,6 @@ if (__DEV__) {
webview: true,
};

validatePropertiesInDevelopment = function (type: string, props: any) {
validateARIAProperties(type, props);
validateInputProperties(type, props);
validateUnknownProperties(type, props, {
registrationNameDependencies,
possibleRegistrationNames,
});
};

// IE 11 parses & normalizes the style attribute as opposed to other
// browsers. It adds spaces and sorts the properties in some
// non-alphabetical order. Handling that would require sorting CSS
Expand All @@ -133,12 +117,25 @@ if (__DEV__) {
// in that browser completely in favor of doing all that work.
// See https://github.com/facebook/react/issues/11807
canDiffStyleForHydrationWarning = canUseDOM && !document.documentMode;
}

warnForPropDifference = function (
propName: string,
serverValue: mixed,
clientValue: mixed,
) {
function validatePropertiesInDevelopment(type: string, props: any) {
if (__DEV__) {
validateARIAProperties(type, props);
validateInputProperties(type, props);
validateUnknownProperties(type, props, {
registrationNameDependencies,
possibleRegistrationNames,
});
}
}

function warnForPropDifference(
propName: string,
serverValue: mixed,
clientValue: mixed,
) {
if (__DEV__) {
if (didWarnInvalidHydration) {
return;
}
Expand All @@ -156,9 +153,11 @@ if (__DEV__) {
JSON.stringify(normalizedServerValue),
JSON.stringify(normalizedClientValue),
);
};
}
}

warnForExtraAttributes = function (attributeNames: Set<string>) {
function warnForExtraAttributes(attributeNames: Set<string>) {
if (__DEV__) {
if (didWarnInvalidHydration) {
return;
}
Expand All @@ -168,12 +167,11 @@ if (__DEV__) {
names.push(name);
});
console.error('Extra attributes from the server: %s', names);
};
}
}

warnForInvalidEventListener = function (
registrationName: string,
listener: any,
) {
function warnForInvalidEventListener(registrationName: string, listener: any) {
if (__DEV__) {
if (listener === false) {
console.error(
'Expected `%s` listener to be a function, instead got `false`.\n\n' +
Expand All @@ -190,11 +188,13 @@ if (__DEV__) {
typeof listener,
);
}
};
}
}

// Parse the HTML and read it back to normalize the HTML string so that it
// can be used for comparison.
normalizeHTML = function (parent: Element, html: string) {
// Parse the HTML and read it back to normalize the HTML string so that it
// can be used for comparison.
function normalizeHTML(parent: Element, html: string) {
if (__DEV__) {
// We could have created a separate document here to avoid
// re-initializing custom elements if they exist. But this breaks
// how <noscript> is being handled. So we use the same document.
Expand All @@ -208,7 +208,7 @@ if (__DEV__) {
);
testElement.innerHTML = html;
return testElement.innerHTML;
};
}
}

// HTML parsing normalizes CR and CRLF to LF.
Expand Down

This file was deleted.

37 changes: 24 additions & 13 deletions packages/react-dom-bindings/src/client/setInnerHTML.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,15 @@
* @flow
*/

/* globals MSApp */

import {SVG_NAMESPACE} from './DOMNamespaces';
import createMicrosoftUnsafeLocalFunction from './createMicrosoftUnsafeLocalFunction';
import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags';

// SVG temp container for IE lacking innerHTML
let reusableSVGContainer: HTMLElement;

/**
* Set the innerHTML property of a node
*
* @param {DOMElement} node
* @param {string} html
* @internal
*/
const setInnerHTML: (
node: Element,
html: {valueOf(): {toString(): string, ...}, ...},
) => void = createMicrosoftUnsafeLocalFunction(function (
function setInnerHTMLImpl(
node: Element,
html: {valueOf(): {toString(): string, ...}, ...},
): void {
Expand Down Expand Up @@ -66,6 +57,26 @@ const setInnerHTML: (
}
}
node.innerHTML = (html: any);
});
}

let setInnerHTML: (
node: Element,
html: {valueOf(): {toString(): string, ...}, ...},
) => void = setInnerHTMLImpl;
// $FlowFixMe[cannot-resolve-name]
if (typeof MSApp !== 'undefined' && MSApp.execUnsafeLocalFunction) {
/**
* Create a function which has 'unsafe' privileges (required by windows8 apps)
*/
setInnerHTML = function (
node: Element,
html: {valueOf(): {toString(): string, ...}, ...},
): void {
// $FlowFixMe[cannot-resolve-name]
return MSApp.execUnsafeLocalFunction(function () {
return setInnerHTMLImpl(node, html);
});
};
}

export default setInnerHTML;
4 changes: 2 additions & 2 deletions packages/react-dom-bindings/src/client/setTextContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {TEXT_NODE} from './HTMLNodeType';
* @param {string} text
* @internal
*/
const setTextContent = function (node: Element, text: string): void {
function setTextContent(node: Element, text: string): void {
if (text) {
const firstChild = node.firstChild;

Expand All @@ -32,6 +32,6 @@ const setTextContent = function (node: Element, text: string): void {
}
}
node.textContent = text;
};
}

export default setTextContent;
Loading

0 comments on commit d310d65

Please sign in to comment.