Add display name after create context #13501
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/47061/ |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit d15543e:
|
| } | ||
|
|
||
| return id; |
There was a problem hiding this comment.
Here id could be any expression. Given the function name, maybe it makes sense to return undefined if it's not an identifier?
There was a problem hiding this comment.
We currently check t.isIdentifier after getDisplayNameReferenceIdentifier. I can move the check here.
| t.isIdentifier(callee.object, { name: "React" }) && | ||
| (t.isIdentifier(callee.property, { name: "createContext" }) || |
There was a problem hiding this comment.
This would also allow React[createContext].
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 4c13d54:
|
existentialism
added
the
PR: New Feature 🚀
| } else if (parentPath.isAssignmentExpression()) { | ||
| // var FooContext = React.createContext() | ||
| const ref = parentPath.node.left; | ||
| parentPath.insertAfter(buildDisplayNameAssignment(ref, id)); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Uh well ok then, I guess assignment expressions in this case are not common.
nicolo-ribaudo
added
the
PR: Ready to be Merged
|
Hi @JLHwung . I am having issues in a different project today and it seems related to this change. The error we are getting: Module parse failed: Unexpected token (6:130)
File was processed with these loaders:
* ./node_modules/cache-loader/dist/cjs.js
* ./node_modules/@docusaurus/core/node_modules/babel-loader/lib/index.js
You may need an additional loader to handle the result of these loaders.
| * This source code is licensed under the MIT license found in the
| * LICENSE file in the root directory of this source tree.
> */import React from'react';const ThemeContext=/*#__PURE__*/React.createContext(undefined);ThemeContext.displayName="ThemeContext"export default ThemeContext;
@ ./node_modules/@docusaurus/theme-classic/lib-next/theme/hooks/useThemeContext.js 6:33-79 6:131-143
@ ./node_modules/@docusaurus/theme-classic/lib-next/theme/Navbar/index.js
@ ./node_modules/@docusaurus/theme-classic/lib-next/theme/Layout/index.js
@ ./node_modules/@docusaurus/theme-classic/lib-next/theme/NotFound.js
@ ./node_modules/@docusaurus/core/lib/client/exports/ComponentCreator.js
@ ./.docusaurus/routes.js
@ ./node_modules/@docusaurus/core/lib/client/clientEntry.jsThe missing Thanks! |
This reverts commit e9bc7c1.
github-actions
bot
added
the
outdated
The PR inserts
context.displayName = "context"assignments afterReact.createContext()call. For common cases like variable declarations and assignment, we reuse the context binding when building assignments. Then we fallback to the sequence expression approach, whereReact.createContext()is transformed into(ref = React.createContext(), ref.displayName = "myContext", ref).