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

Manipulating nodes within forEachChild or forEachDescendant causes out of date nodes to be wrapped #359

Closed
dsherret opened this issue Jul 12, 2018 · 2 comments
Labels

Comments

@dsherret
Copy link
Owner

Obvious why this is happening, just have to find a solution:

const project = new Project({ useVirtualFileSystem: true });
const sourceFile = project.createSourceFile("test.ts", `
const t = 5;

function test() {};`);

sourceFile.forEachDescendant(node => {
    if (TypeGuards.isVariableDeclaration(node))
        node.setType("any");
    console.log("Kind: " + node.getKindName());
    console.log("Text: " + node.getFullText());
});

Outputs:

Kind: VariableStatement
Text:
const t = 5;
Kind: VariableDeclarationList
Text:
const t = 5
Kind: VariableDeclaration
Text:  t: any = 5
Kind: Identifier
Text:  t
Kind: AnyKeyword
Text:  any
Kind: NumericLiteral
Text:  5
Kind: FunctionDeclaration
Text:  = 5;

function test
Kind: Identifier
Text: ction
Kind: Block
Text: est
Kind: EmptyStatement
Text: (
Kind: EndOfFileToken
Text:
@dsherret dsherret added the bug label Jul 12, 2018
@cancerberoSgx
Copy link
Contributor

Interesting couple of questions / thoughts:

do you think this is an issue that the library should tackle or should the user be responsible of first query all the nodes he wants to modify and modify them all together as suggested in the docs ? Thinking very roughly I don't see way of solving the problem besides visiting all source file's nodes and fix their ranges which could be very expensive. Will check it out how / if is solved for other modifications like addClass/addMethod, etc I think in those there is no such problem . I would like to know what's your plan for this if you have one and is short to explain :)

What about making the whole sourcefile (forgotten) when user modify any of its nodes as it happens with insertText() ?

Also, perhaps it could related with #351 - if I have a way to identify nodes uniquely then something like the following should be safe: ids.forEach(id=>sourceFile.query(id).modify()) -

(sorry for thinking aloud here)

@dsherret
Copy link
Owner Author

The node tracking between manipulations is one of the main features of this library and what makes it so easy to use.

For performance reasons, it's for this specific situation:

If the code analysis is using types, symbols type checker, or program, then a large performance improvement can be gained by doing an initial analysis of the code first, then afterwards carrying out the manipulations.

Anyway, this is a really easy fix. Will do it soon.

@dsherret dsherret changed the title Manipulating nodes within forEachKind of forEachDescendant causes out of date nodes to be wrapped Manipulating nodes within forEachChild or forEachDescendant causes out of date nodes to be wrapped Jul 12, 2018
dsherret added a commit that referenced this issue May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants