Skip to content

Remove local getComponentName from ReactPartialRenderer and use shared version #11999

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from
Closed

Remove local getComponentName from ReactPartialRenderer and use shared version #11999

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 11 additions & 17 deletions packages/react-dom/src/server/ReactPartialRenderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import memoizeStringOnly from 'fbjs/lib/memoizeStringOnly';
import warning from 'fbjs/lib/warning';
import checkPropTypes from 'prop-types/checkPropTypes';
import describeComponentFrame from 'shared/describeComponentFrame';
import getComponentName from 'shared/getComponentName';
import {ReactDebugCurrentFrame} from 'shared/ReactGlobalSharedState';
import {
REACT_FRAGMENT_TYPE,
Expand Down Expand Up @@ -73,8 +74,7 @@ if (__DEV__) {

describeStackFrame = function(element): string {
const source = element._source;
const type = element.type;
const name = getComponentName(type);
const name = getComponentName(element);
const ownerName = null;
return describeComponentFrame(name, source, ownerName);
};
Expand Down Expand Up @@ -130,12 +130,6 @@ const newlineEatingTags = {
textarea: true,
};

function getComponentName(type) {
return typeof type === 'string'
? type
: typeof type === 'function' ? type.displayName || type.name : null;
}

// We accept any tag to be rendered but since this gets injected into arbitrary
// HTML, we want to make sure that it's a safe tag.
// http://www.w3.org/TR/REC-xml/#NT-Name
Expand Down Expand Up @@ -187,7 +181,7 @@ function warnNoop(
if (__DEV__) {
const constructor = publicInstance.constructor;
const componentName =
(constructor && getComponentName(constructor)) || 'ReactClass';
(constructor && getComponentName({type: constructor})) || 'ReactClass';
Copy link
Collaborator

Choose a reason for hiding this comment

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

This part is kinda hacky and is the reason they weren’t shared. It’s just a different data structure. Maybe it’s reasonable to change the function itself to accept the type directly, but I would rather have duplication than “lie” about the type like here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I had the hacky reservation while making the change. I’m open to changing the shared function to just accept the type or I can close this PR. Doesn’t bother me either way, just let me know how you want to proceed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let’s just keep it how it is? I don’t see us changing this function anyway so it’s not worth unifying.

const warningKey = `${componentName}.${callerName}`;
if (didWarnAboutNoopUpdateForComponent[warningKey]) {
return;
Expand Down Expand Up @@ -348,14 +342,14 @@ function createOpenTagMarkup(
return ret;
}

function validateRenderResult(child, type) {
function validateRenderResult(child, element) {
if (child === undefined) {
invariant(
false,
'%s(...): Nothing was returned from render. This usually means a ' +
'return statement is missing. Or, to render nothing, ' +
'return null.',
getComponentName(type) || 'Component',
getComponentName(element) || 'Component',
);
}
}
Expand All @@ -370,6 +364,7 @@ function resolve(
let element: ReactElement;

let Component;
let componentName;
let publicContext;
let inst, queue, replace;
let updater;
Expand All @@ -393,6 +388,7 @@ function resolve(
break;
}
publicContext = processContext(Component, context);
componentName = getComponentName(element) || 'Unknown';

queue = [];
replace = false;
Expand Down Expand Up @@ -427,8 +423,6 @@ function resolve(
Component.prototype &&
typeof Component.prototype.render === 'function'
) {
const componentName = getComponentName(Component) || 'Unknown';

if (!didWarnAboutBadClass[componentName]) {
warning(
false,
Expand All @@ -444,7 +438,7 @@ function resolve(
inst = Component(element.props, publicContext, updater);
if (inst == null || inst.render == null) {
child = inst;
validateRenderResult(child, Component);
validateRenderResult(child, element);
continue;
}
}
Expand Down Expand Up @@ -500,7 +494,7 @@ function resolve(
child = null;
}
}
validateRenderResult(child, Component);
validateRenderResult(child, element);

if (typeof inst.getChildContext === 'function') {
childContextTypes = Component.childContextTypes;
Expand All @@ -510,7 +504,7 @@ function resolve(
invariant(
contextKey in childContextTypes,
'%s.getChildContext(): key "%s" is not defined in childContextTypes.',
getComponentName(Component) || 'Unknown',
componentName,
contextKey,
);
}
Expand All @@ -519,7 +513,7 @@ function resolve(
false,
'%s.getChildContext(): childContextTypes must be defined in order to ' +
'use getChildContext().',
getComponentName(Component) || 'Unknown',
componentName,
);
}
}
Expand Down