Missing function signatures in typescript.d.ts

[arocheleau]

TypeScript Version: 4.0.2

Search Terms: getMutableClone cloneNode createThrow createNew NodeFactory

Code

ts.factory.cloneNode(someNode);

Expected behavior:

The compiler should compile the code just fine without error.

Actual behavior:

The compiler signal an error saying that the cloneNode function does not exists on NodeFactory.

On ts.getMutableClone the deprecated notice says we should use factory.cloneNode instead. Unfortunately, there is no cloneNode function defined on NodeFactory according to the declaration file (typescript.d.ts).

However, if I trick the compiler with the following code, I see that a cloneNode function do exist on the object that implements the NodeFactory interface at runtime.

(ts.factory as any).cloneNode(someNode);

Related Issues:

I have seen a few deprecated notices that are a bit misleading on many functions in the ts namespace. I have not checked all functions, but to name a few:

• createThrow says to use factory.createThrow instead of factory.createThrowStatement.
• updateThrow says to use factory.updateThrow instead of factory.updateThrowStatement.
• ...
• The same goes for createNew, updateNew, createTry, updateTry

[longlho]

same with createObjectLiteral, createCall, createArrayLiteral as well :( nvm they're renamed, the comment seems to be inaccurate then

[brad4d]

I hit this today while trying to update tsickle to use the new NodeFactory API.
I need to use nodeFactory() to replace getMutableClone(), but cannot.
The misleading method names were annoying, but I was able to find the ones I needed.

Is this likely to be fixed in a 4.0.x release?
I'm tempted to put off the whole migration of tsickle to NodeFactory until this is fixed so I don't end up with partially migrated code.

[rbuckton]

getMutableClone wasn't safe to use, in general, since AST nodes should be treated as if they are immutable. I don't recommend using cloneNode as isn't all that safe to use either, and I've been trying to reduce its use within the compiler itself. You're generally better off using one of the update methods if you need to get an updated Node. The most of the uses of cloneNode in the compiler today are to essentially clone identifiers for unique comment/source map positions, which I intend to clean up.

What are your use cases for getMutableClone that aren't served by the various update methods?

[rbuckton]

I'd like to further clarify why cloneNode shouldn't be used and isn't available in the published API. In the TypeScript compiler we track the set of necessary transformations on a Node using a bitmask known as TransformFlags. Prior to 4.0, we would dynamically compute the transform flags for a Node during bind and whenever returning from a call to visitNode or visitNodes, however we've had multiple issues in the past with flags not being calculated in certain cases.

When we refactored the node factory API so that we could use it in the parser, we changed the behavior so that we calculate these transform flags as the tree is created. The logic behind how these flags are calculated is now baked into the various create... (and update...) methods on the factory. If you previously used getMutableClone to copy a node so that you could replace a child (which was the case in several places in the compiler), transformation flags would no longer be correctly calculated. While cloneNode does copy the transform flags of the source node, if you were to attempt to use it in the fashion I just described it would result in incorrect emit. Since this is a practice we'd like to discourage (which is why most child nodes in the TypeScript AST are now readonly), we elected to not make cloneNode available as part of our public API.

The only safe use of cloneNode would be to clone a keyword, token, identifier, or literal. If we do make it public, it will likely only be available for use with those specific cases.

[rbuckton]

I just put up a PR that fixes the messages, though it does not expose cloneNode as I still need further information on existing getMutableClone use cases.

[longlho]

So the use case that we have to it is effectively ease of use: With cloneNode we can freely manipulate its children regardless of type & depth while with update* API it seems like we've have to return a new visitor that does the traversal & update separately. Coupled that w/ the fact that if we want to pass some parent state to the visitor we've have to hoist/nest it.

In general I've already rewritten our suite of transformers to the update* API so it's ok :) It certainly makes transformer maintenance a little trickier.

The babel AST manipulation is a little more ergonomic in this aspect.

[edsrzf]

Here's a use case for getMutableClone/cloneNode that I've had:

https://github.com/airbnb/ts-migrate/blob/8c1f8c1e48d43f7bc6f9e03cf7f8ba743ac5257c/packages/ts-migrate-plugins/src/plugins/member-accessibility.ts#L80-L84

I have a ClassElement of some kind and I want to update only its accessibility modifier. Without cloning, I have to figure out which specific type of class element I have (PropertyDeclaration, MethodDeclaration, GetAccessor, or SetAccessor) and call its respective update function. It's doable, but it's significantly more verbose and less convenient.

[arocheleau]

In my case, the fact that the old API had a getMutableClone function make me doubt that the AST nodes were truly immutable and so I was cautious when it was time to mutate nodes. My knowledge of the compiler internals is limited. I have built it from the few documentations I found, some blog posts, code examples, trials and errors and code tracing. Most of the documentation I found was minimal and refered to older versions, so it was probably outdated.

Anyway, in my application, I am cloning nodes at 2 places. One of the 2 place is already using the update* functions and so, I guess I may safely remove the call to the cloning function since, the way I understand it, the update* functions are returning new immutable nodes.

For the other place, I am unaware if I must clone the node or not. Without getting into details, let say that I am creating a tool for internal usage that generates a single dts file to expose our public API. I needed some private fields and functions to be removed from the dts since they are refering types that are not publicly available. Furthermore, most of theses types came from or were imported from packages that once bundled, are not accessible to the consumer. Basically, with the help of the TypeChecker, I identify the types that need to be part of our public API, extract their AST nodes, transform them and put them all in a single SourceFile.

I had an issue when I asked the Printer to generate the source code of this SourceFile made of AST nodes coming from different files. For example, the Printer did not generate the values of the enum members. After tracing the code, I figured out that the Printer tried to read the enum member values directly from the original source file and failed instead of consulting the AST nodes which have everything it needs.

In order to fix this issue, I created a transformer that removes all links or informations about the original source file. So I invoke the following functions on the nodes:

• ts.setOriginalNode
• ts.setTextRange
• ts.setSourceMapRange
• ts.setCommentRange

I don't know if these functions work like the update* functions and return a new immutable node or if it mutates it. Right now, I am just cautious and clone the node first.

Thank you