Expression Structures

[dsherret]

From #681.

Perhaps structures should be expanded to include expressions as well. Currently they only include declarations.

This is something to consider for the future though... still a lot of work to do elsewhere.

[LeonardKoch]

Thanks for giving the expressions some more attention. This would allow me to move away from doing a lot of this with just strings. Until you get to this, is there a better way of adding expressions to modules or functions than strings? So if for example inside a function I want to add the following code is there any more elegant way than just using addStatements with a string of code passed in? Haven't found anything in the docs or when digging through the typings.

someFunctionWeWantToCall(
    5,
    someOtherFunction(1, 2, 3)
)

Thanks!

[dsherret]

@LeonardKoch nope. The only real way is to use add/insertStatements at the moment. It will probably be a while before I get to this issue.

[GrandSchtroumpf]

Hello, Any idea what kind of interface to expect for each structure ? For example should an ObjectLiteralExpression be :

interface ObjectLiteralStructure {
  kind: Synthax.ObjectLiteralExpression;
  value: Record<string, Structure>;
}

Or

interface ObjectLiteralStructure {
  kind: Synthax.ObjectLiteralExpression;
  properties: { key: IdentifierStructure, value Structure }[];
}

Or something else...

[dsherret]

@GrandSchtroumpf it would be more like the second one. The key might be a computed expression (ex. { [someCall()]: 5 }) so the structure wouldn't be able to use a string key.

[GrandSchtroumpf]

Ok thanks. I'm just creating the "node -> structure" logic right now, I will focus on the "structuer -> node" logic later. Here is my starting work :

export function getStructure(node: Node<ts.Node>) {
  switch (node.getKind()) {
    case SyntaxKind.StringLiteral: return getStringStructure(node as any);
    case SyntaxKind.ArrayLiteralExpression: return getArrayStructure(node as any);
    case SyntaxKind.ObjectLiteralExpression: return getObjectStructure(node as any);
    case SyntaxKind.Identifier: return getIdentifierStructure(node as any);
    
    case SyntaxKind.ArrowFunction: return getArrowFunctionStructure(node as any);
    case SyntaxKind.FunctionExpression: return getFunctionExpressionStructure(node as any);

    // Supercharge current structure with expressions from above
    case SyntaxKind.Decorator: return getDecoratorStructure(node as any);
    case SyntaxKind.ClassDeclaration: return getClassDeclarationStructure(node as any);
  }
}

export const getIdentifierStructure = (node: Identifier) => ({
  kind: node.getKind(),
  value: node.getText()
});

export const getArrayStructure = (node: ArrayLiteralExpression) => ({
  kind: node.getKind(),
  elements: node.getElements().map(element => getStructure(element))
});
...

Am I on the right direction ? Also how can I know all the missing node structure ? Finally I saw the SynthaxKind doesn't match the StructureKind. Why is it like that ?

[ajvincent]

I decided to bite the bullet and roll my own: "type structures" (work in progress)

The idea behind my approach is to support an additional property of existing structures, typeStructure:

export interface TypedNodeTypeStructure extends TypedNodeStructure
{
  typeStructure: TypeStructure | undefined;
}

With a couple of helper classes (TypeWriterManager) and a set of opinionated structure classes, I think I can cover the majority of cases.

One explicit exclusion is an ObjectLiteralTypedStructure interface. Although I imagine them as Omit<InterfaceDeclarationStructure, "name" | "kind">, writing code to serialize that exactly the same as ts-morph does would be non-trivial, and for my use case, not worth the effort.

I'm aiming to keep this backwards-compatible with ts-morph, so if someone wants to port it to ts-morph directly, they could start with this.

[ajvincent]

I'm nearly done with my own implementation, but I've hit a snag, a very expensive one.

export interface ImplementsClauseableNodeStructure {
  implements?: (string | WriterFunction)[] | WriterFunction;
}

Two of those characters, [], require me to implement JavaScript proxies to handle this correctly. (Users can set numeric properties as they want.) Long story short, I've spent three frantic days trying to support this when a much simpler solution could exist:

export interface ImplementsClauseableNodeStructure {
  implements?: (string | WriterFunction)[] | Set<string | WriterFunction> | WriterFunction;
}

If I could implement the ClassDeclarationStructure's implements property as a Set, then I have a much simpler path forward.

@dsherret would you accept a patch to ts-morph to make this possible?

[ajvincent]

Someone recently wrote an article encouraging developers to write throw-away prototypes as a first draft. (Thank you, Slashdot, for the link.)

I couldn't agree more. Though I'm pretty sure he didn't mean over 7,500 lines of prototype, not counting tests or documentation. Nevertheless, here it is.

At this point, I'd like to open a conversation about porting this into ts-morph. From admittedly minimal testing, it does appear to work. I'd really like to see something like this as a npm package under ts-morph, say, ts-morph/structures.

Such an effort clearly is going to require a proper design discussion. I really would like that discussion to begin, with this as a starting point.

Alternatively, I could release this on npm as a stand-alone package, but I would give it a different name to ensure it clearly wasn't from ts-morph. Say, @ajvincent/ts-morph-structures.

I will be using these structure classes in my own project for certain, so I expect to iron out bugs as I run across them.

Please?

[dolanmiu]

@ajvincent/ts-morph-structures

Are you planning to publish @ajvincent/ts-morph-structures to npm by any chance? I need this feature!

[ajvincent]

Are you planning to publish @ajvincent/ts-morph-structures to npm by any chance? I need this feature!

TLDR: I hope not - I really want this done right in ts-morph itself, if at all possible. I'm very willing to drive an organic implementation merge request, once there's a good high-level design approval to do so.

---

Releasing my own package is a backup plan, under the scenario where my prototype meets an absolute "wontfix" rejection. There are several reasons I want to avoid this:

• If I publish this as a separate package, and then ts-morph does release something similar, then I'd be saying to users "okay, don't use my package anymore, use the official ones from ts-morph." This isn't a nice thing to do to users who are also fellow developers.
• There's a fair amount of maintenance:
  • TypeScript puts out a significant release roughly every three months
  • ts-morph probably has to do releases to keep pace with TypeScript
  • I would have to do releases to keep pace with both.
• What my code does is less efficient (and probably prone to bugs)
  • ts-morph has to generate structures, which involves a node tree traversal
  • then I have to traverse the node tree again, and the structure tree I get from ts-morph, and weave them together to replace the type fields on structures (an inherently fragile task)
  • whereas, as a part of ts-morph itself, the work could be done once, correctly, using internal API's
• The dependence on JavaScript proxies alone is a serious code smell for most developers - and I understand that.
  • Especially incomplete proxies.
• More eyeballs here means we have more people familiar with and able to work on fixes.
• None of my code has been reviewed yet, by anyone. Nor has my design.

I cannot emphasize enough: this is a prototype. I need this feature too, but I don't want to put out something bad to other users which then becomes either obsolete (or worse, irreplaceable).

For the same reasons, I strongly encourage no one else to fork my project, jump the gun and put out their own release.

I'm trying to take a deep breath right now, after spending a very frantic month putting this together in my spare time. I want to give @dsherret and other stakeholders some time to evaluate this. I only finished the first draft yesterday.

[ajvincent]

@dsherret I realized a couple days ago I didn't write up a plan to implement this. Here is the design plan for your feedback and approval.

[dsherret]

So at this point this would be a pretty massive breaking change and maintenance burden. I'm not sure it should be added. I'd recommend just traversing the AST rather than using structures for this.

[dsherret]

Going to close this one because I don't think it's viable to do anymore considering the massive breaking change it would be.

[LeonardKoch]

Thanks for giving it consideration and closing the issue once a decision was made!

[ajvincent]

Well, I'm still moving forward with my own ts-morph-structures supporting package, and it's nearly at version 1.0. https://github.com/ajvincent/ts-morph-structures/

[ajvincent]

For anyone still watching this thread, I have good news.

• https://www.npmjs.com/package/ts-morph-structures
• https://github.com/ajvincent/ts-morph-structures/
• https://ajvincent.github.io/ts-morph-structures/

I've published version 1.0.1 of this new library package, to provide structure classes correctly implementing ts-morph's structures. I also have "type structure" classes to reflect type nodes, and MemberedTypeToClass to help convert interfaces to classes. Compatible with TypeScript 5.4 and ts-morph 22.