JSX conditionals show error

[benzitohhh]

In js2-jsx-mode (and js2-mode)...

Simple JSX expressions work ok, for example:

function a() {
  return (
    <div>{expression}</div>
  );
}

But the below conditional JSX statement is shown with a "missing ) in parenthetical" errorr:

function b() {
  return (
    <div>
      {condition && <div></div>}
    </div>
  );
}

[dgutov]

@jacksonrayhamilton Also ping.

[jacksonrayhamilton]

To fix this we need parsing support for JSX.

@benzitohhh, my suggestion in the meantime is to disable js2-mode linting and use a linter program like ESLint with the Flycheck package:

In your init file:

(require 'flycheck)
(require 'js2-mode)

(add-to-list 'auto-mode-alist '("\\.jsx?\\'" . js2-jsx-mode))

(setq js2-mode-show-parse-errors nil)
(setq js2-mode-show-strict-warnings nil)

;; Disable JSCS linting (optional but if you're using ESLint you probably don't
;; need this).
(let ((checkers (get 'javascript-eslint 'flycheck-next-checkers)))
  (put 'javascript-eslint 'flycheck-next-checkers
       (remove '(warning . javascript-jscs) checkers)))

(defun setup-js2-mode ()
  (flycheck-select-checker 'javascript-eslint)
  (flycheck-mode))

(add-hook 'js2-mode-hook #'setup-js2-mode)

In a file called .eslintrc in the root of a project directory (or your home directory):

{
  "ecmaFeatures": {
    "jsx": true
  }
}

@dgutov, how would you like to handle this? Disable linting inside JSX blocks until parsing support is added?

[dgutov]

how would you like to handle this? Disable linting inside JSX blocks until parsing support is added?

Can we even do that reliably? Disable showing errors inside E4X literals, but still show them outside?

[jacksonrayhamilton]

Perhaps I shouldn't be absolute about "needing JSX parsing support." There might be another way around this that involves improving or adapting the E4X code.

Can we even do that reliably? Disable showing errors inside E4X literals, but still show them outside?

I'll have to take another look at the code, but I was thinking of binding some variable for the duration of parsing E4X that would prevent errors from being recorded.

[dgutov]

There might be another way around this that involves improving or adapting the E4X code.

Might be. But my first reaction is also "needs smarter JSX parser".

I'll have to take another look at the code, but I was thinking of binding some variable for the duration of parsing E4X that would prevent errors from being recorded.

The thing to worry about is whether certain parse errors might make the parser think that the E4X literal ends much later in the buffer than it actually does. But yes, it can work.

[benzitohhh]

@jacksonrayhamilton ok thanks for the workaround

[jacksonrayhamilton]

Another thing to consider: Maybe dedicated linter programs aren't just a "workaround," but are actually the best way forward.

I don't use js-mode's linting, because I believe the 3rd-party solutions (JSLint, JSHint, JSCS, ESLint...) are superior. They are highly featureful, modernized and configurable. They also work outside of Emacs, which is a dealbreaker if you work on a team with a diverse ecosystem of editors, but you all need to maintain the same code quality standards.

Delegating linting elsewhere would also be a relief. We could remove quite a few options from the Customize interface, probably hundreds of lines of code, and close out #260, #237, #225, and #29.

[dgutov]

Would you delegate parsing to an external program as well?

Or do you propose to continue parsing the buffer contents in Elisp, but never show parsing errors? That's definitely not ideal.

[benzitohhh]

@jacksonrayhamilton Ah ok cool, will definitely look into using a linter

[jacksonrayhamilton]

Would you delegate parsing to an external program as well?

I think that'd also be a good idea.

Or do you propose to continue parsing the buffer contents in Elisp, but never show parsing errors? That's definitely not ideal.

Having an AST is still useful for other purposes, like finding nodes, refactoring and coloring--things that are best done in the editor. We just don't have to set the expectation that we also do error checking, because that can be done independently.

[dgutov]

I think that'd also be a good idea.

That would be #288.

We just don't have to set the expectation that we also do error checking, because that can be done independently.

If the AST has errors, it can be incorrect, including places arbitrarily far after the error. If we don't signal the error somehow, the user might wonder why sometimes an AST-based operation fails.

[jacksonrayhamilton]

If the AST has errors, it can be incorrect, including places arbitrarily far after the error. If we don't signal the error somehow, the user might wonder why sometimes an AST-based operation fails.

Maybe we could just drop warnings, and still report errors. Users could still have feedback OOTB, and maintainers could enjoy most of the reliefs I mentioned earlier. Although it won't automatically solve this issue. :stuck_out_tongue_winking_eye:

[dgutov]

You can already disable all warnings via custom variables, but, like you said, this issue is about a parse error.

[jacksonrayhamilton]

I am having second thoughts about adding js2-jsx-mode. I think adding it may have been selfish. I misfortunately became the maintainer of JSX code and wanted to be able to easily indent it. However, I ignored the old proverb:

If the camel once gets his nose in the tent, his body will soon follow.

And now the scope of this JavaScript mode will creep to non-standard extensions, which is a bad idea in terms of maintenance and forwards-compatibility.

It's rather outrageous that Facebook coerces developers to use non-standard extensions as a means to stomach their otherwise-"ugly" React.js API. But what's even more disappointing is that developers take the bait.

I have come under the suspicion that JSX is fundamentally a bad thing, because it mixes the concerns of HTML with JavaScript. It appeals to the emotions of web developers, but validates their fixation on a less-than-ideal application presentation layer, which will prevent the web platform from improving. And it's certainly unnecessary, even if one is in the business of generating HTML.

So, I'm thinking now we ought to remove JSX support, for the betterment of the JavaScript and web development community, and for the sake of setting reasonable expectations for this mode. It is unfortunate to provide indentation support just to yank it away a few weeks later, but the benefits of setting a policy against scope creep should outweigh the (perceived) inconvenience in the long run.

[dgutov]

It doesn't matter much to me one way or the other, but let's face it, whether we support JSX or not, that won't have a significant impact on JSX's popularity. The main result will be that Emacs users that do work with it, will have to suffer broken indentation, or use other editors for this task.

So, I'm thinking now we ought to remove JSX support, for the betterment of the JavaScript and web development community, and for the sake of setting reasonable expectations for this mode.

Do you mean only js2-jsx-mode, or js-jsx-mode too?

It is unfortunate to provide indentation support just to yank it away a few weeks later, but the benefits of setting a policy against scope creep

Considering we've split them into separate major modes, I wouldn't really call it scope creep.

[jacksonrayhamilton]

The main result will be that Emacs users that do work with it, will have to suffer broken indentation, or use other editors for this task.

This can be solved by not using JSX. Lack of support should incite users to reject it.

Do you mean only js2-jsx-mode, or js-jsx-mode too?

Both.

Considering we've split them into separate major modes, I wouldn't really call it scope creep.

Despite an organizational strategy, it's still an influx of features: Syntax highlighting, parsing, errors / warnings, comments, niceties like sgml-close-tag... And they need to be maintained, e.g. indentation is failing for me now due to a JSX inside JS inside JSX false positive.

Will it be worth all this effort so that users can write HTML inside their JS? I'd say no. I thought I could ease my suffering by adding indentation support for JSX, but it seems I'm only preparing us (maintainers and users) for more suffering.

[dgutov]

Lack of support should incite users to reject it.

In industrial environment, on average, you don't make most of the choices. They're either made collectively (and Emacs is not the most popular editor for JS), or by other people long before you.

Then the user's choice is to pick another editor or use web-mode. Does that sound good to you?

Also recall that until you contributed the JSX indentation support, people were using js2-mode with JSX anyway. It was just a lot less convenient.

Both.

I would be against removing js-jsx-mode, unless it is demonstrably broken in a significant fraction of JSX files. Some support is better than nothing.

Syntax highlighting, parsing, errors / warnings, comments, niceties like sgml-close-tag... And they need to be maintained, e.g. indentation is failing for me now due to a JSX inside JS inside JSX false positive.

Failing indentation in some cases is unfortunate, but otherwise I'm perfectly happy to only provide indentation support, until someone else very clever comes along and contributes the rest.

[jacksonrayhamilton]

Then the user's choice is to pick another editor or use web-mode. Does that sound good to you?

Those aren't the only choices. A person has the power to reject complexity. A superior can make the decision to not use something, and an inferior can petition against it. But if we continue to support this thing, users have a means to cope with their fate, and there will be less a chance of anyone doing anything. That sounds bleak to me.

Whether people already show signs of helplessness is irrelevant to me. The popularity of Emacs is irrelevant. The creation of a thousand forests is in one acorn. We should reject this, and tell our users to do so too. Programmers have to realize that the deletion of features can be just as valuable as their addition.

[dgutov]

But if we continue to support this thing, users have a means to cope with their fate

They still have that means: use js2-mode or js-mode, and get broken indentation, or use web-mode, and get nonstandard behavior in other respects (and also lose benefits provided by js2-mode).

This reminds me of the last year's Emacs vs. Clang discussion: https://lwn.net/Articles/583390/

RMS similarly argued that since Clang undermines the power GCC, being copyleft, afforded to FSF, Emacs shouldn't hurry to use it. You can see for yourself how well that discussion went. LLVM didn't suddenly lose in popularity either.

[jacksonrayhamilton]

This quote stood out to me:

To make GCC available for such use would be throwing in the towel. If that enables GCC to "win", the victory would be hollow, because it would not be a victory for what really matters: users' freedom.

RMS regards the rights of users to be of greater importance than the features offered by non-free compilers. If I should be a steward to the users and the codebase, then I should stand in opposition.

The languages those compilers support (notably C++) are so complicated that the availability of a tool to analyze them is a point of contention. Another takeaway from that discussion is it wouldn't have happened in the first place if programmers chose simpler languages. Programmers can at least choose JS over JSX, so that's what I'd like to encourage.

[dgutov]

I agree with the sentiment, but there is a point at which protecting users' rights by taking away capabilities becomes an oxymoron.

Here's a quote that rings closer to reality, IMHO: https://lists.gnu.org/archive/html/emacs-devel/2015-01/msg00171.html

But anyway, while I disagree, I'm perfectly happy to delegate that decision to someone else, e.g. Emacs' current maintainer.

@jwiegley, what do you think?

[jwiegley]

I loved the camel proverb, btw, I will have to remember that one.

Can we fork the jsx support into its own add-on module, on top of but not part of js2-mode?

haskell-mode recently went through the problem of incorporating far too many features in what should have remained a core set of functionality, rather than breaking the new features into separate addons. I don't think we should disallow or discourage JSX use, but having it be separate from js2-mode development sounds like it would be preferable.

[dgutov]

I don't think we should disallow or discourage JSX use, but having it be separate from js2-mode development sounds like it would be preferable.

I would say the same, but actually, it's already separate: we've added js-jsx-mode to the core (in js.el) during the current development cycle, which basically inherits from js-mode and uses a specialized indentation function.

So js2-jsx-mode basically has a 3-line definition. We've just copied the indentation code to js2-old-indent.el as well, so that users of Emacs < 25 could also benefit from it. Packaging that separately seems like an overkill to me.

[ustun]

@jacksonrayhamilton I do agree with you regarding JSX, but it is impossible to stop its popularity and JS2 mode has to support it one way or another.

That said, I created an example repo that we can all point people at for using React without JSX: https://github.com/ustun/react-without-jsx Putting this here in case I can change some people's minds.

[jacksonrayhamilton]

Hey, wanted to apologize for getting so zealous about this. I've realized that it does not really matter if I think JSX is a "good" thing or not. It exists, and more people are using it, so it is relevant that there be support for it.

The weight that a feature has with respect to being non-standard is a function of the mode's extensibility. So I'm returning to the belief that JSX is not necessarily the issue, rather it would be the parser, as we originally concluded.

[11111000000]

ping

[dgutov]

ping

Patch welcome.

[felipeochoa]

I've taken a first crack at implementing a parser for JSX in a separate package. The very rough first version is here. Though the parser is not built for modularity, I was able to plug in the different XML parser by simply advising 'js2-parse-xml-initializer.

The parser correctly handles cases like the one highlighted in this issue as well as ternary expressions inside JSX bracketed expressions (even with crazy nesting). This is definitely not production-ready (it will hang sometimes on malformed/partially formed expressions), but the fontification and elimination of spurious syntax errors is quite nice. I think it will also be straightforward to check for unbound tag names once I figure out how scopes work in js2.

[dgutov]

@felipeochoa This is great. When you figure out the hangs, feel free to open a pull request to merge that code into js2-mode. We could start with a simple switch based on user option whether to parse XML as E4X or JSX.

Although to be frank, keeping it as an external package should work just as well: one advice won't affect the performance to any perceptible degree.

[felipeochoa]

I think the parser is stable now! One odd thing I noticed is that when I use it with js2-jsx-mode a few indentation issues I used to have (e.g., after => and ?:) seem to be fixed. Is that expected? I thought the indentation code did not rely on the AST.

There are a few things I haven't done since I haven't fully grokked the Rhino/js2 parser code, so would appreciate some pointers/help:

• Is the node-printing code used anywhere? What does that function actually do?
• How does scope handling and tracking of undeclared variables work? It would be nice to check that the JSX labels are defined somewhere
• What exactly does the js2-compiler-xml-available variable do? Should rjsx set it to t?

PS: Sorry to commandeer this issue, but it seemed like the most active JSX-related place

[dgutov]

One odd thing I noticed is that when I use it with js2-jsx-mode a few indentation issues I used to have (e.g., after => and ?:) seem to be fixed. Is that expected? I thought the indentation code did not rely on the AST.

Not really. Do you have an example?

Is the node-printing code used anywhere?

It's used in the parser tests (see tests/parser.el), but probably not much else.

What does that function actually do?

Which function?

What exactly does the js2-compiler-xml-available variable do? Should rjsx set it to t?

It's used in js2-parse-property-access. And it's t by default.

Sorry to commandeer this issue, but it seemed like the most active JSX-related place

You could open a new one. It's fine, though.

[benzitohhh]

@felipeochoa and @dgutov wow thanks this looks amazing - will it be available on melpa soon?

[felipeochoa]

@benzitohhh thanks! I need to add tests and generally vet it more (currently doing a lot of React so that won't be a problem) before putting it up there. Maybe in a week or two. If you're interested in being an alpha tester though, you're welcome to download it and try it out -- I haven't had any issues with it since the last batch of commits.

[benzitohhh]

OK thanks will have a look

On 23 Oct 2016, at 15:25, Felipe notifications@github.com wrote:

@benzitohhh thanks! I need to add tests and generally vet it more (currently doing a lot of React so that won't be a problem) before putting it up there. Maybe in a week or two. If you're interested in being an alpha tester though, you're welcome to download it and try it out -- I haven't had any issues with it since the last batch of commits.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[felipeochoa]

rjsx-mode is stable now and live on MELPA. It's now a major mode deriving from js2-jsx-mode. It fixes this issue, and I think also #351, #330, and #140.

[ustun]

Thank you very much @felipeochoa

[jacksonrayhamilton]

Nice work, @felipeochoa.

Have you considered merging what is now in rjsx-mode into js2-jsx-mode in this repository? Or, would @dgutov now be open to moving js2-jsx-mode to a separate package, such that what is now in rjsx-mode is incorporated into js2-jsx-mode, and all JSX-related support is provided externally?

It seems like a bit much to have all these different modes providing different pieces of the puzzle.

[felipeochoa]

Thanks! I'm fine with either approach -- I'd rather give rjsx-mode a bit of time as an independent package since I'm still working on the parser to improve some mal-formed expression handling (to avoid painting the whole buffer red when a user starts adding an element). Once that stabilizes (help/ideas/bike-shedding welcome, btw!) I'd love to move that into js2 (if you'll have it!)

[dgutov]

Once that stabilizes (help/ideas/bike-shedding welcome, btw!) I'd love to move that into js2 (if you'll have it!)

Sounds good to me.

[benzitohhh]

@felipeochoa rjsx is amazing - thanks for making it happen

[felipeochoa]

I'm +10 on this change! I've already signed the Emacs CLA, and save for a couple of fairly self contained PRs have written all the rjsx code so it should be easy legally to add to Emacs. Happy to relicense my code under GPL or whatever is necessary here.

Unfortunately won't be able to do the merging myself, but by all means go ahead here!

Thanks for cleaning up the JSX Emacs mess :)

-Felipe

On Mon, Jun 3, 2019, 9:38 PM Jackson Ray Hamilton notifications@github.com wrote:

@felipeochoa https://github.com/felipeochoa In light of recent improvements to Emacs’ built-in support for JSX, in this post https://github.com/mooz/js2-mode/pull/523#issuecomment-496288390 I resurfaced the notion of integrating RJSX’s extensions to the JS2 parser into this repository.

It would be more valuable now for RJSX’s parsing to be a part of JS2, since JS2’s inheritance of some of the new features provided by js-mode feels contingent on more comprehensive support for JSX being available here (i.e., also being able to lint it and highlight it).

I know you’ve expressed that your time is more limited lately. If you aren’t available to undertake this task now, but if you also are at least in agreement on the value of it, then we could plan for it and elicit help from generous volunteers.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mooz/js2-mode/issues/292?email_source=notifications&email_token=AAU4Y4AI4PWCCW7LROBOQWLPYXIQPA5CNFSM4BT5PLV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW3HJUA#issuecomment-498496720, or mute the thread https://github.com/notifications/unsubscribe-auth/AAU4Y4DL2QDGZU3BRMY3NQLPYXIQPANCNFSM4BT5PLVQ .

[jacksonrayhamilton]

I originally left a comment here and you must have received an email about that, but I deleted the comment right away because I decided it’d be better to create a new issue without necromancing and inheriting the weight of this issue.

Here’s that comment I deleted:

@felipeochoa In light of recent improvements to Emacs’ built-in support for JSX, in this post I resurfaced the notion of integrating RJSX’s extensions to the JS2 parser into this repository.

It would be more valuable now for RJSX’s parsing to be a part of JS2, since JS2’s inheritance of some of the new features provided by js-mode feels contingent on more comprehensive support for JSX being available here (i.e., also being able to lint it and highlight it).

I know you’ve expressed that your time is more limited lately. If you aren’t available to undertake this task now, but if you also are at least in agreement on the value of it, then we could plan for it and elicit help from generous volunteers.

In any case, message received! Thanks Felipe.

[apoorv569]

I am having this problem as well. This issue was opened in 2015 is it still not fixed?

[dgutov]

If you've read the comments, the recommendation was to use rjsx-mode.

And now you can also install a pre-release build of Emacs 29 compiled with tree-sitter, build js grammar, and try js-ts-mode. Or typescript-ts-mode, in case that's your thing.

[apoorv569]

If you've read the comments, the recommendation was to use rjsx-mode.

And now you can also install a pre-release build of Emacs 29 compiled with tree-sitter, build js grammar, and try js-ts-mode. Or typescript-ts-mode, in case that's your thing.

Ah! I missed that. Thanks rjsx-mode seems nice.