File node

[sebmck]

So in order for Program to qualify for visiting in the common node-parent pattern, a File node is required in order to wrap Program so it can be visited. This is a node used by Babel and recast etc. Would there be any objection to including this in the ESTree specification? It doesn't necessarily have to be compulsory and can be purely optional.

[michaelficarra]

I don't understand this request at all.

[sebmck]

How about asking for clarification then? You have a visitor. You need to traverse a tree. If you want to visit Program then it needs to have a parent. File would be that parent. What are you specifically confused about?

[michaelficarra]

I've written a few different kinds of visitors for trees, and I have never had to introduce an extra node to visit the root node. Why does a node have to have a parent to be visited?

[sebmck]

Have you never had to replace the root node?

[RReverser]

I'm also not sure it's required as proper solution on spec level. If you add yet another root node, you just wrap the old problem and get new one as one day you might want to replace the File node as well. It might be handy to define such node for specific visitor pattern inside of your own tool, but not worth to be added to standard IMO.

[sebmck]

@RReverser There's no reason to touch File as it's only an implementation detail.

[RReverser]

There's no reason to touch File as it's only an implementation detail.

Just like Program now.

Have you never had to replace the root node?

Also - I know this was addressed to @michaelficarra, but hadn't such experience really. Usually either manipulating body property or creating completely new program and returning it was enough.

Can you please show any example where having yet another top-level wrapper would be necessary?

[sebmck]

@RReverser

Just like Program now.

Not really. Program is documented and in order to visit it your visitors need to be able to visit root nodes with no parent which is pretty nasty IMO.

Also - I know this was addressed to @michaelficarra, but hadn't such experience really. Usually either manipulating body property or creating completely new program and returning it was enough.

That's if Program can be visited at all. The concept of visiting a node without a parent (ie. no contextual reference) seems incredibly antiquated to me, especially with an AST as contextual as ESTree. Special casing visiting a root node of Program with no parent doesn't really fly well with me.

Also I'm not really trying imposing this on anyone, just wanting to get feedback. I'd rather not have to make an AST superset if possible so was just probing whether or not anyone else sees the merit in standardising it.

[nzakas]

ESLint uses the visitor pattern and is able to visit the Program node. I haven't encountered a need for a root node above it in order to accomplish this.

👎 I think this is really just an implementation detail and shouldn't be part of the spec.

[RReverser]

Program is documented

Isn't that what you're asking for File in this issue? Thus it turns from implementation detail to another documented node that visitors need to be aware of and that doesn't have own parent.

[sebmck]

Ok closing this as the consensus seems to be against this. To address the remaining points:

@RReverser

Isn't that what you're asking for File in this issue? Thus it turns from implementation detail to another documented node that visitors need to be aware of and that doesn't have own parent.

It's not comparable since File is only necessary to facilitate alternate visitor implementations that don't visit their root nodes on traversal and instead require parents to maintain context.

@nzakas

ESLint uses the visitor pattern and is able to visit the Program node. I haven't encountered a need for a root node above it in order to accomplish this.

Not really comparable. Completely different requirements and ESLint rule visitors don't even get access to the parent unless they manually get it via context.getAncestors().pop() (AFAIK).

When you're doing code transforms there's usually a lot of metaprogramming to reduce repetition and increase stability so having the AST be very neutral (eg. always having a parent, always being able to replace the current node from the parent one with just a key reference) is extremely beneficial as it increases code reuse and having to check constantly for the existence of parent is a code smell IMO.

[getify]

I'm just curious, can you explain in what use case(s) would you need to replace the Program node? I see why having to not special-case in that respect would be nice, but not sure when I'd ever need to replace the entire Program with another Program?

[sebmck]

@getify When you're replacing a set of nodes from a map of others. Having to special case root nodes and replace their props is bizarre.

The main reason for the File node is the strict node parent requirement that I impose on visitors that I mentioned in previous comments.

[michaelficarra]

@sebmck In my visitors, I usually pass in a list of ancestors. An empty list for the root node's ancestors is just fine.

[sebmck]

Also thanks for the discussion guys, I'll just leave it as footnote in the Babel docs or something justifying it's existence.

@michaelficarra It's mostly due to one of the requirements of my visitor pattern where it's very strict about relationships so you can accurately resolve a node. I've abstracted a node's position in the tree with a "path", very similar to the one in ast-types.

The biggest benefit of this that I've found, is that it allows you to easily do type inference, constant folding, dead code elimination and more on an AST easily without any other intermediary format (SSA etc). This is mostly because you can "jump" around the AST easily just by referencing these paths since they're always absolute as they react to tree changes.

[getify]

Having to special case root nodes and replace their props is bizarre.

I was asking, when would you need to do this to the Program node? What attributes of it specifically would change, where it would be easier to just replace the Program node?

[sebmck]

@getify The entire thing. You may be dealing with transforms that don't care what nodes they touch (minification etc, there are many scenarios I've found where transforms are node-agnostic). Or even more commonly, you may just be dealing with Program and BlockStatement nodes (only node types that contain a list of statements) where it's simpler to replace the entire node instead of mangling with properties in the event that one of them might be a Program.

[RReverser]

The main reason for the File node is the strict node parent requirement that I impose on visitors that I mentioned in previous comments.

Note that ESTree doesn't define parent as well, so both parent and File are implementation-specific properties / types that surely can exist, but not necessarily need to be standardized.

[sebmck]

@RReverser parent has nothing to do with AST definitions though? File clearly does.

[RReverser]

You pointed to parent as part of reasoning behind File, so transitionally it becomes implementation / use-case specific thing, too.

[sebmck]

@RReverser I was pointing at that it's not unreasonable to need the parent of a node to establish context. This is a pretty big philosophical concept of ESTree that has been discussed numerous times before and is even in the philosophy section of the README:

Contextless: Nodes should not retain any information about their parent. ie. a FunctionExpression should not be aware of if it's a concise method. (eg. #5)

[RReverser]

Sure, but "I don't want to check parent" is not really a good argument for creating separate node type as this violates

Unique: Information should not be duplicated. ie. a kind property should not be present on Literal if the type can be discerned from the value. (eg. #61)

In this case it's less obvious, but still File doesn't bring any new information that can't be retrieved from Program itself and is only a "virtual" node type that only helps to simplify code for specific use-cases.

[sebmck]

Sure, but "I don't want to check parent" is not really a good argument
for creating separate node type as this violates

That was not the crux of my reasoning. It fundamentally pollutes how my
visitors are architectured since a parentless node makes no sense.

Unsure why the points I raised are still being debated when I already
submitted that it's not a big deal if it's not in the spec. I raised this
issue to get consensus so I can justify having a superset.

In this case it's less obvious, but still File doesn't bring any new
information that can't be retrieved from Program itself and is only a
"virtual" node type that only helps to simplify code for specific use-cases.

I wasn't using that to justify the existence of a File node, I think I've
already established why it's necessary in my case. I was using the ESTree
philosophy to justify the tight coupling to parents in my visitor pattern.

On Friday, 12 June 2015, Ingvar Stepanyan notifications@github.com wrote:

Sure, but "I don't want to check parent" is not really a good argument
for creating separate node type as this violates

Unique: Information should not be duplicated. ie. a kind property should
not be present on Literal if the type can be discerned from the value. (eg.
#61 #61)

In this case it's less obvious, but still File doesn't bring any new
information that can't be retrieved from Program itself and is only a
"virtual" node type that only helps to simplify code for specific use-cases.

—
Reply to this email directly or view it on GitHub
#91 (comment).

Sebastian McKenzie