atom / language-javascript

JavaScript language package for Atom
Other
194 stars 236 forks source link

Optional chaining not supported in embedded script #640

Open pmrotule opened 5 years ago

pmrotule commented 5 years ago

Prerequisites

Description

Optional chaining is currently supported in Javascript files, but not in embedded script (<script> tag in HTML). It is currently treated as a ternary operator which breaks the detection of the closing tag </script> since a colon is expected.

I first reported the issue to language-vue to finally realized it was the case for all embedded scripts: https://github.com/hedefalk/atom-vue/issues/97

51517628-a99b7a80-1e1b-11e9-8465-c8dec87ee902

Steps to Reproduce

  1. Open a HTML file
  2. Within a <script> tag, use the optional chaining operator (i.e. window.test?.something)

Expected behavior: Closing script tag detected

Actual behavior: Closing script tag not detected

Reproduces how often: Always

Versions

Atom: ~1.32.2~ upgraded to 1.34.0 OS: macOS Mojave version 10.14.3 (18D109)

Aerijo commented 5 years ago

The latest Stable version is 1.34.0; can you see if the issue occurs on that?

pmrotule commented 5 years ago

@Aerijo I'm now on 1.34.0 and the issue still occurs.

Aerijo commented 5 years ago

Oh, actually nothing in Atom "supports" the optional chaining operator (which is still just a proposal?)

One possiblity you see is it just recovering from an error and guessing it's a ternary operator or something. Inspecting the tree, it shows image

Without something to copy, I can only guess the cause of your issue. Most likely is you have Tree-sitter parsers disabled (so the above doesn't apply), and the TextMate grammar is looking for the end of the ternary operation (leaks like this are a known issue with the older TextMate grammars, and are "grandfathered" in by the desire to match the TextMate editor's behaviour).

pmrotule commented 5 years ago

I understand that it is still just a proposal, but since it is now supported by Babel and more and more people are using it, it would be great that Atom supports it as well. Moreover, language-javascript is currently supporting optional chaining, so I consider it as an issue if it works in Javascript files, but not within <script> tag in HTML files (or .vue file in my case).

I am unfortunately not familiar with debugging Atom and I don't fully understand your explanation about TextMate grammar (how do you "inspect the tree"?). ~I'm also not sure how come you have javascript highlighting in an "untitled" file (I personally have no highlighting until I save it as a .js file)~. Anyway, the issue I'm referring to is only happening within <script> tag in HTML files.

Aerijo commented 5 years ago

That PR was closed, not merged. Optional chaining is not supported yet.

rsese commented 5 years ago

I understand that it is still just a proposal, but since it is now supported by Babel and more and more people are using it, it would be great that Atom supports it as well.

For other things in early stages, we've established that knowing what version of ECMAScript this would land in is our bar of inclusion e.g. see https://github.com/atom/language-javascript/pull/588 (which was for a stage 3 proposal, looks like optional chaining is earlier at stage 1).

Edit: this is also not parsed by Tree-sitter so https://github.com/tree-sitter/tree-sitter-javascript would also need to be updated

molvqingtai commented 4 years ago

Now Optional-Chaining is at Stage 4, TypeScript 3.7 has been officially added, and I hope to add support for him in the future

pieczorx commented 4 years ago

Any updates?

jellevandevelde commented 4 years ago

It has been over a year since this ticket was opened. Any updates or timeline yet?

mattgrosso commented 4 years ago

I agree, I'd love to see a resolution to this issue.

pmrotule commented 4 years ago

bump

Brugarolas commented 4 years ago

Any updates?

veitbjarsch commented 4 years ago

Do we have any updates here?

raphaelparent commented 3 years ago

Bumping for updates.

Also is there a package that enables this in the meantime?

mattgrosso commented 3 years ago

Mostly I run into this issue when I'm styling a single file Vue component. After months of being annoyed it finally occurred to me that I can side step this issue by moving my style tag above my script tag. Not a real solution but it's making my life better in the meantime and I though I'd share just in case doing this might help someone else.

veitbjarsch commented 3 years ago

@mattgrosso I've just overcome the issue by installing language-babel. This comes with the support of the latest ECMA features.

mattgrosso commented 3 years ago

@mattgrosso I've just overcome the issue by installing language-babel. This comes with the support of the latest ECMA features.

Thank you! That did the trick.

chfritz commented 3 years ago

Using language-babel instead of language-javascript is not a solution of course and language-babel doesn't work with tree-sitter.

icecream17 commented 3 years ago

I think https://github.com/atom/language-javascript/pull/686 would theoretically work if you deselect "use tree sitters" in settings, and downgrade the tree-sitter version to 12. Currently the tree-sitter fixes don't work, but the non-tree-sitter parser does work.

Currently the pr is blocked and I'm not working on it.