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.

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

Add display name after create context #13501

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
166 changes: 116 additions & 50 deletions packages/babel-plugin-transform-react-display-name/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,111 @@ import { declare } from "@babel/helper-plugin-utils";
import path from "path";
import { types as t } from "@babel/core";

export default declare(api => {
api.assertVersion(7);

function addDisplayName(id, call) {
const props = call.arguments[0].properties;
let safe = true;

for (let i = 0; i < props.length; i++) {
const prop = props[i];
const key = t.toComputedKey(prop);
if (t.isLiteral(key, { value: "displayName" })) {
safe = false;
break;
}
function addDisplayNameInCreateClass(id, call) {
const props = call.arguments[0].properties;
let safe = true;

for (let i = 0; i < props.length; i++) {
const prop = props[i];
const key = t.toComputedKey(prop);
if (t.isLiteral(key, { value: "displayName" })) {
safe = false;
break;
}
}

if (safe) {
props.unshift(
t.objectProperty(t.identifier("displayName"), t.stringLiteral(id)),
);
if (safe) {
props.unshift(
t.objectProperty(t.identifier("displayName"), t.stringLiteral(id)),
);
}
}

function getDisplayNameReferenceIdentifier(
path: NodePath<t.CallExpression>,
): ?t.Node {
let id;

// crawl up the ancestry looking for possible candidates for displayName inference
path.find(function (path) {
if (path.isAssignmentExpression()) {
id = path.node.left;
} else if (path.isObjectProperty()) {
id = path.node.key;
} else if (path.isVariableDeclarator()) {
id = path.node.id;
} else if (path.isStatement()) {
// we've hit a statement, we should stop crawling up
return true;
}

// we've got an id! no need to continue
if (id) return true;
});

// ensure that we have an identifier we can inherit from
if (!id) return;

// foo.bar -> bar
if (t.isMemberExpression(id)) {
id = id.property;
}

return id;
Copy link
Member

Choose a reason for hiding this comment

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

Here id could be any expression. Given the function name, maybe it makes sense to return undefined if it's not an identifier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We currently check t.isIdentifier after getDisplayNameReferenceIdentifier. I can move the check here.

}

function isCreateContext(node) {
let callee;
return (
t.isCallExpression(node) &&
t.isMemberExpression((callee = node.callee)) &&
t.isIdentifier(callee.object, { name: "React" }) &&
(t.isIdentifier(callee.property, { name: "createContext" }) ||
Copy link
Member

Choose a reason for hiding this comment

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

This would also allow React[createContext].

t.isStringLiteral(callee.property, { value: "createContext" }))
);
}

function buildDisplayNameAssignment(ref, displayName) {
return t.assignmentExpression(
"=",
t.memberExpression(t.cloneNode(ref), t.identifier("displayName")),
t.stringLiteral(displayName),
);
}

function addDisplayNameAfterCreateContext(
id,
path: t.NodePath<t.CallExpression>,
) {
const { parentPath } = path;
if (parentPath.isVariableDeclarator()) {
// FooContext = React.createContext()
const ref = parentPath.node.id;
// parentPath.parentPath must be a VariableDeclaration because getDisplayNameReferenceIdentifier
// does not support patterns
parentPath.parentPath.insertAfter(buildDisplayNameAssignment(ref, id));
} else if (parentPath.isAssignmentExpression()) {
// var FooContext = React.createContext()
const ref = parentPath.node.left;
parentPath.insertAfter(buildDisplayNameAssignment(ref, id));
Copy link
Contributor Author

@JLHwung JLHwung Jun 23, 2021

Choose a reason for hiding this comment

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

Inserting after an assignment expression does not preserve completion records.
e.g.

(context = React.createContext("light"))
// transformed to
(context = React.createContext("light"), context.displayName = "context")

Maybe we should only insert for variable declarations.

Copy link
Member

Choose a reason for hiding this comment

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

Inserting after a variable declaration changes the completion record too.

This is a general problem with insertAfter and it's not just in this PR. I think we should check it separately checking in insertAfter if the completion record matters (e.g. if a parent is a do expression) and injeting a temp variable similarly to what we do for insertAfter in expressions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current version of do expression forbids a do block ending with variable declarations: https://tc39.es/proposal-do-expressions/#sec-doexpression-static-semantics-early-errors, but we haven't implemented that in Babel parser. Guess it should be okay for variable declarations (most common cases) here?

Copy link
Member

Choose a reason for hiding this comment

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

Uh well ok then, I guess assignment expressions in this case are not common.

} else {
// (ref = React.createContext(), ref.displayName = "id", ref)
const ref = path.scope.generateUidIdentifier("ref");
path.replaceWith(
t.sequenceExpression([
t.assignmentExpression("=", ref, path.node),
buildDisplayNameAssignment(ref, id),
t.cloneNode(ref),
]),
);
}
}

const createContextVisited = new WeakSet();

export default declare(api => {
api.assertVersion(7);

const isCreateClassCallExpression =
t.buildMatchMemberExpression("React.createClass");
const isCreateClassAddon = callee => callee.name === "createReactClass";
Expand Down Expand Up @@ -66,44 +148,28 @@ export default declare(api => {
displayName = path.basename(path.dirname(filename));
}

addDisplayName(displayName, node.declaration);
addDisplayNameInCreateClass(displayName, node.declaration);
}
},

CallExpression(path) {
const { node } = path;
if (!isCreateClass(node)) return;

let id;

// crawl up the ancestry looking for possible candidates for displayName inference
path.find(function (path) {
if (path.isAssignmentExpression()) {
id = path.node.left;
} else if (path.isObjectProperty()) {
id = path.node.key;
} else if (path.isVariableDeclarator()) {
id = path.node.id;
} else if (path.isStatement()) {
// we've hit a statement, we should stop crawling up
return true;
if (isCreateClass(node)) {
const id = getDisplayNameReferenceIdentifier(path);
// identifiers are the only thing we can reliably get a name from
if (t.isIdentifier(id)) {
addDisplayNameInCreateClass(id.name, node);
}
} else if (isCreateContext(node)) {
if (createContextVisited.has(node)) {
return;
}
createContextVisited.add(node);
const id = getDisplayNameReferenceIdentifier(path);
// identifiers are the only thing we can reliably get a name from
if (t.isIdentifier(id)) {
addDisplayNameAfterCreateContext(id.name, path);
}

// we've got an id! no need to continue
if (id) return true;
});

// ensure that we have an identifier we can inherit from
if (!id) return;

// foo.bar -> bar
if (t.isMemberExpression(id)) {
id = id.property;
}

// identifiers are the only thing we can reliably get a name from
if (t.isIdentifier(id)) {
addDisplayName(id.name, node);
}
},
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ThemeContext = React.createContext("light");
ThemeContext.displayName = "CustomThemeContext";
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ThemeContext = React.createContext("light");
ThemeContext.displayName = "ThemeContext";
ThemeContext.displayName = "CustomThemeContext";
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ThemeContext = React.createContext("light");
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ThemeContext = React.createContext("light");
ThemeContext.displayName = "ThemeContext";
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
var enhancedContext = qux(React.createContext("light"));
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
var enhancedContext = qux((_ref = React.createContext("light"), _ref.displayName = "enhancedContext", _ref));
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
({
ThemeContext: React.createContext("light")
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
({
ThemeContext: (_ref = React.createContext("light"), _ref.displayName = "ThemeContext", _ref)
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"plugins": ["transform-react-display-name"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
var ThemeContext = React.createContext("light");
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
var ThemeContext = React.createContext("light");
ThemeContext.displayName = "ThemeContext"