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

Add JSX Syntax #860

Closed
wbond opened this issue Mar 29, 2017 · 11 comments
Closed

Add JSX Syntax #860

wbond opened this issue Mar 29, 2017 · 11 comments
Labels
significant T: enhancement Improvement of existing language features

Comments

@wbond
Copy link
Member

wbond commented Mar 29, 2017

We'd like to support JSX out of the box to provide a high-quality JS Goto Definition experience for users of React

@Thom1729
Copy link
Collaborator

Thom1729 commented Apr 3, 2017

Is the goal to add JSX to the existing JavaScript package, or as a separate package? Or to replace the JavaScript syntax with a more full-featured one? I ask because, frustrated with the limitations of the built-in package and some issues with babel-sublime, I've nearly completed a new JS + JSX + Flow syntax.

@wbond
Copy link
Member Author

wbond commented Apr 3, 2017

It would be a separate syntax, but if possible will include contexts from the existing JavaScript syntax. I am not sure if it would make sense to include in the current JavaScript package, or a new package.

We won't be replacing the existing JavaScript syntax, but instead fixing bugs and improving it over time. It got a big rewrite in 2016, and we wouldn't want to do another breaking change since the new syntax follows the scope naming guidelines for the most part.

It seems @ccampbell (who contributed a bunch towards the JavaScript syntax in 2016) has recently added a JSX package to Package Control that is based on top of the existing JavaScript syntax. I haven't looked at it yet, but that would probably be a reasonable place to start from. https://packagecontrol.io/packages/Simple%20JSX

In my opinion the biggest thing will be coming up with a good set of syntax tests to prevent regressions as we develop it.

@aziz
Copy link
Contributor

aziz commented Apr 3, 2017

Please take a look at Sublime Ecmascript syntax as well. It has a really good support for JSX.

The only downside of it might be that it fails too early and marks a whole bunch of the view as invalid. For me it's not a big deal but I've seen others complaining about a similar thing here.

In terms of completeness, correctness and granularity of scopes, Sublime Ecmascript is phenomenal. For example using it you can differentiate {} when used for normal JS statements like if and switch from the ones that define an object or the ones that are used for functions or classes, the same thing is true for (). These things can not be done with the built-in JS syntax or any another popular JS syntax definition on package control (like babel-sublime). Unfortunately the author is not abiding to the new scope naming rules we're trying to promote here and I don't see it being merged to the default packages repo but it could be used as a perfect reference.

Note that Sublime Ecmascript syntax is 5038 lines of code compared to only 1476 lines for the official one here.

@ccampbell
Copy link
Contributor

@aziz that actually came up before and the author chimed in on the discussion in #96 too.

I happen to think the current native sublime JavaScript syntax in Sublime is pretty good right now. I am saying that as someone who writes JavaScript pretty much every day. From what I can tell, the main things it seems to be lacking are support for some ES7 features, but I haven’t been writing ES7 so that hasn’t bothered me.

The problem I have with the other syntaxes (which I probably got into a bit on that PR too) is that none of them use the proper scopes to play nicely with out of the box themes in Sublime so they all ship with custom themes.

I am not a member of the Sublime team or anything, but I think the best path forward is submitting bug reports to this repo for inconsistencies/missing feature in the native JavaScript highlighting. I am going to do that now

@Thom1729
Copy link
Collaborator

Thom1729 commented Apr 3, 2017

I saw Sublime Ecmascript, and I had the same concerns that you do: way too much code and not up-to-date with the syntax guidelines. Also, no JSX. But I took some inspiration from that package.

Currently, my syntax is 1425 lines for full ES7 and Stage 2 support (minus regexes, which are separate), plus JSX, plus most of Flow. It handles automatic semicolon insertion and differentiates various uses of punctuation (comma operator vs comma punctuation, braces for blocks vs function body, etc). It's also designed to be modular so that you could compile versions with or without JSX/Flow. Finally, it runs about 15% faster than the built-in syntax (and over 80% faster than sublime-babel).

However, it relies on a custom macro system for concision and readability, and it uses a very different architecture from the built-in syntax definitions. (The scopes need a bit of updating to match the existing syntax, and it could use more comprehensive test coverage, but these are fixable.)

It may still be possible to incorporate some of the features into the existing syntax, if not the performance gains.

@max-mykhailenko
Copy link

It will be great. Babel syntax doesn't maintaining now. Do you plan to support flow too?
Thanks!

@wbond
Copy link
Member Author

wbond commented Oct 17, 2017

This should probably happen after #1009

@Thom1729
Copy link
Collaborator

It should be a fairly simple extension of #1009. I used such an extension as an example for my YAML Macros package. As soon as #1009 is merged I could adapt that patch.

The downside is that without using macros, there's no graceful way to extend the JavaScript syntax other than copy-pasting it and applying changes in-place. This would be a pain from a maintenance perspective; future changes would have to be manually merged, and there's every possibility that the two syntaxes could become desynchronized.

Alternatively, we could build JSX support directly into the base JavaScript syntax. In practice, this probably shouldn't be problematic, but it's an ugly solution.

Alternatively alternatively, I could write a third-party package with a full-featured JSX extension using YAML Macros. With #1009 merged, it could directly reference the built-in syntax, so it would automatically build against the latest version. The obvious drawback of this is that a third-party syntax isn't an out-of-the-box solution, which was the whole point of this issue.

Alternatively3, a built-in macro system for the core would solve all of these problems, but that's a project with a much larger scope and broader implications.

@Thom1729
Copy link
Collaborator

Thom1729 commented Jan 8, 2019

I should note that the hypothetical system described in the previous comment now exists as JS Custom. JSX is an option that can be freely enabled or disabled. It's a third-party package, so it isn't built in, but it may nevertheless be an appropriate solution for many users.

@michaelblyons
Copy link
Collaborator

Close with 40ec1f2?

@wbond
Copy link
Member Author

wbond commented Aug 18, 2020

Good call @michaelblyons!

@wbond wbond closed this as completed Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
significant T: enhancement Improvement of existing language features
Projects
None yet
Development

No branches or pull requests

6 participants