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

Include JSX Extension #44

Open
sebmarkbage opened this issue Mar 6, 2015 · 24 comments
Open

Include JSX Extension #44

sebmarkbage opened this issue Mar 6, 2015 · 24 comments

Comments

@sebmarkbage
Copy link

I thought I'd open the discussion around move the JSX extension to the ESTree repo. It could go into an experimental extensions section since it is not part of the spec nor any active proposal. However, it is fairly stable and implemented by several downstream parsers.

Currently this extension lives in the JSX spec repo:

https://github.com/facebook/jsx/blob/master/AST.md

It would be convenient for tooling implementors to have a single place to look at.

@sebmck
Copy link

sebmck commented Mar 6, 2015

I'm not opposed to this. Another thing that this brings up is how to handle copyright and licensing in ESTree.

@RReverser
Copy link
Member

I think it would be still the best to keep extension specs in separate repos under corresponding owners, otherwise we might end with maintaining a lot of JS dialects in one centralized repo instead of "modular" approach that separate dedicated repos give.

@nzakas
Copy link
Contributor

nzakas commented Mar 6, 2015

In my opinion, once an extension becomes ubiquitous, it would be nice to have in this repo (like the appendices in ECMA-262).

@RReverser
Copy link
Member

once an extension becomes ubiquitous

Do you think we should include type annotations then? Which version if so (TypeScript? Flow? ...?)

@nzakas
Copy link
Contributor

nzakas commented Mar 6, 2015

I don't, because everyone is doing it differently right now.

For JSX, any parser that implements it is implementing the same form, so it's already a pseudo-standard.

@RReverser
Copy link
Member

For JSX, any parser that implements it is implementing the same form, so it's already a pseudo-standard.

I never argued that it's a standard, as well as TypeScript has own one, and as of now both of them live in own curated specs, so I'm just proposing to leave things that way.

Personally, I also contributed to JSX spec and 100% interested in it's support and further evolution, just saying that ESTree itself should cover official ECMAScript only IMO.

@nzakas
Copy link
Contributor

nzakas commented Mar 6, 2015

::shrugs:: From a parser implementer point of view, I'd prefer having things in one spot. I also don't feel super strongly about, so I'd defer to consensus.

@sebmck
Copy link

sebmck commented Mar 7, 2015

@RReverser

I think it would be still the best to keep extension specs in separate repos under corresponding owners, otherwise we might end with maintaining a lot of JS dialects in one centralized repo instead of "modular" approach that separate dedicated repos give.

Not sure if this is what you're directly proposing but what about having a estree/jsx repo?

@mikesherov
Copy link
Contributor

I agree with @sebmck that it opens ESTree to licensing copyright questions... which isn't so bad, but needs answering.

@mikesherov
Copy link
Contributor

I'm also fine with having the JSX extension living here. Seems as if we can evaluate individual extensions with

@sebmck sebmck mentioned this issue Mar 9, 2015
@dead-claudia
Copy link
Contributor

dead-claudia commented Mar 9, 2015 via email

@sebmck
Copy link

sebmck commented Mar 12, 2015

Can we have a consensus on this? I'm 👍 on having it in a separate repo.

@mikesherov
Copy link
Contributor

I'd prefer like @nzakas to have it here, so I still want to look into licensing for a bit before we come to an answer.

@sebmck
Copy link

sebmck commented Mar 12, 2015

@mikesherov In the core repo or in a separate one? I only suggest a separate repo because if JSX is included then I'd like to write one up for flow too (of the current esprima-fb structure and with @jeffmo's blessing of course) and it's arguably the slippery slope bought up by @RReverser .

@mikesherov
Copy link
Contributor

I was thinking more along the lines of a directory here marked extensions or some much. I don't care much either way, so if you guys feel strongly about separate repo same org, let's do it :-)

@jeffmo
Copy link

jeffmo commented Mar 12, 2015

I would kind of prefer separate-dir/same-repo just to help things stay in sync with official lang updates.

@RReverser
Copy link
Member

@jeffmo What makes it "not in sync" when in separate repo?

My preference is still to keep vendor-specific extensions under their vendors. As a user (developer), I'd rather expect to find JSX AST spec right near the JSX syntax spec in facebook/jsx than under estree/estree/blob/master/extensions/jsx.

If others don't consider this as an option, then let's at least define extensions in a separate repo - that would allow to keep clear scope and commit history of this repo as ECMAScript-only.

@sebmck
Copy link

sebmck commented Mar 12, 2015

@RReverser Extension specs may be tightly coupled to the core spec and it's weird/hard to juggle between different repos.

@jeffmo
Copy link

jeffmo commented Mar 12, 2015

@RReverser: Nothing, it's just harder to notice disjointness when the two things don't sit together. Syntax extensions are always coupled with the spec they're extending. It's doable...it'll just require more logistics to keep things up to date, etc. I'm basically just asking that we make it easier to keep the two specs in linear synchronization.

Another future option I have in mind here is that we can consider moving the ESTree spec to be built in an ast-types format in the future (@benjamn has talked about this a bit, and it's a pretty neat idea). This allows the spec to be machine readable. At that point, it's possible to write scripts to validate definitions, AST structures are well-formed and consistent, etc. If we were go to this route, it would also be possible to split JSX off in to another repo and simply write it's machine-readable specs as dependent on a particular version of ESTree...but now you have intra-spec versions to deal with when you make decisions on how to reference the JSX spec. The logistics at this point basically dovetail into similar conversations about having parsers that allow syntactic plugins. Its possible (conceptually), but logistically it's going to be tricky when changes occur.

@RReverser
Copy link
Member

@sebmck

  1. It's not harder/easier/different than juggling between files in the same repo. In any case you open both specs in separate browser tabs while working.
  2. JSX is not an example of "tight coupling" IMO, it perfectly integrates into any of ES5/ES6.
  3. Extensions that are tightly coupled to spec may choose to fork this repo, and to maintain it just like parser forks are (it will be even easier, as spec changes not as frequently as parser code).

@sebmck
Copy link

sebmck commented Mar 12, 2015

@RReverser That seems crazy unnecessary. How many ESTree/SpiderMonkey extensions can you think of that would justify such fragmentation?

@RReverser
Copy link
Member

@sebmck I don't - that's why I didn't understand your "tight coupling" argument and added #3 just in case as a solution for "even if there are such extensions ...".

@sebmck
Copy link

sebmck commented Mar 12, 2015

@RReverser There's the potential for tight coupling. For example, I want to write a Flow AST spec, it's tightly coupled to the way function parameters, classes etc work and future nodes will likely need to be extended.

@RReverser
Copy link
Member

@sebmck I don't see how are they coupled here, as you will be most likely extending base interfaces like Pattern and others instead of each possible production, but even if they're, see argument #1 and tell what the difference it makes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants