This repository has been archived by the owner. It is now read-only.

Document AST differences from ESTree #41

Merged
merged 2 commits into from Jun 22, 2016

Conversation

Projects
None yet
4 participants
@nene
Copy link
Contributor

nene commented Jun 18, 2016

Based on comments in #40

@jmm

This comment has been minimized.

Copy link
Member

jmm commented Jun 18, 2016

@nene Thanks for the effort on this. I'm definitely +1 on noting the AST output format and linking to the spec. I don't think the README is the ideal place to detail differences from ESTree (or whatever AST spec) though. I think that should be in a separate document, probably not linked from the README -- maybe from the Babel AST spec. That document should maybe be a wiki, since I don't know that anyone maintaining Babel / Babylon is going to maintain that information.

If there is going to be a differences from ESTree document (especially if it's committed somewhere) I think we should say something like differences include so that we're not setting the expectation that it's exhaustive or up to date. Even ESTree isn't static -- they make additive changes.

@hzoo

This comment has been minimized.

Copy link
Member

hzoo commented Jun 18, 2016

@nene

This comment has been minimized.

Copy link
Contributor Author

nene commented Jun 18, 2016

Currently the README tells me that Babylon is based on Acorn (which does follow ESTree), so I would assume Babylon also follows ESTree spec unless told otherwise.

Given that README describes input parameters, I'd expect that it also describes the output.

@jmm

This comment has been minimized.

Copy link
Member

jmm commented Jun 18, 2016

Babylon is indeed based on Acorn, but it's a fork and one of the reasons for forking was to be able to diverge from ESTree. I totally agree that we should say what the output format is and link to the AST spec, I just don't think we should inline too much information in the README, like a breakdown of the differences from ESTree. If there's going to be a breakdown we can link to it from the AST spec or somewhere fitting. I'm going to propose the following:

  1. We decide in which repo the AST spec should live. In #40 @loganfsmyth mentioned the possibility of moving it from the babel repo to here. I've thought about that before but been on the fence about it. Right now I lean toward keeping it where it is since multiple core packages under the Babel umbrella and in the babel repo utilize that AST format.
  2. Whichever repo that is, we consider creating a wiki there to break down the differences from ESTree. Alternatively perhaps someone who wants to maintain it creates a repo / gist / whatever for it in userland and we link to it from the AST spec or wherever. I'm not real enthusiastic about the idea of committing it to one of the Babel repos because I'm doubtful that anyone will want to or actually maintain it or that there's even going to be a lot of users interested in it.
  3. We update this README to say that the output is Babel AST format and link to the spec.

@kittens kittens merged commit 55d47ab into babel:master Jun 22, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kittens

This comment has been minimized.

Copy link
Contributor

kittens commented Jun 22, 2016

We should move the AST docs from the babel/babel repo into here. This section in the README is really good though, thank you @nene! Super appreciated

@hzoo

This comment has been minimized.

Copy link
Member

hzoo commented Jun 22, 2016

Made #46

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.