id in import attributes should not be referenced#13733
Conversation
isReferenced should return false for identifiers used in import attributes
| case "ObjectMethod": | ||
| // @ts-expect-error todo(flow->ts) params have more specific type comparing to node | ||
| if (parent.params.includes(node)) { | ||
| return false; |
There was a problem hiding this comment.
Before this PR we checked key and params and return true if node is neither in params nor in key. But the return true branch is unreachable, unless the input node does not have parental relationships with parent.
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/48560/ |
|
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 a9335f3:
|
| if (parent.value === node) { | ||
| return !grandparent || grandparent.type !== "ObjectPattern"; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
When can the node be neither the key nor the value?
There was a problem hiding this comment.
Good question! node could be neither key nor the value when parent is not the actual parent of node. This is actually ill-defined behaviour of t.isReferenced(node, parent): It returns true if the parent.type is not handled by current logic and return false if handled since the conditions may never holds. Unless we change the interface to isReferenced(NodePath), I don't think we can mitigate such issues.
I have updated ObjectProperty handling to align with other types such as ClassProperty.
t.isReferencedreturnstruefor id in import attributes, e.g. thetypeinimport json from "./foo.json" assert { type: "json" }This PR starts from
(todo) flow->tscleanups. Found this bug when reading source.Also simplified the class elements handling. All the
fall throughannotations are removed.