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

Improve commenting of JSX #21

Closed
dortamiguel opened this issue Mar 3, 2018 · 8 comments
Closed

Improve commenting of JSX #21

dortamiguel opened this issue Mar 3, 2018 · 8 comments

Comments

@dortamiguel
Copy link

dortamiguel commented Mar 3, 2018

I detected some problems with jsx comments.

Uncomment doesn't work

If I comment one line and then uncomment it, some curlybraces are being leaved behind.

before
image
after
image
should be
image

Indentation spaces shouldn't be included on the comment

before
image
after
image
should be
image

Inner component is not commented if indentation spaces are not selected

If you select a component like in the next screenshot the comment shortcut doesn't work
image

Use normal javascript comments instead of jsx when the code is inside parenthesis ()

before
image
after
image
should be
image

The javascript that Im using for this examples

function HelloWorld({
  greeting = "hello",
  greeted = '"World"',
  silent = false,
  onMouseOver
}) {
  if (!greeting) {
    return null;
  }

  // TODO: Don't use random in render
  let num = Math.floor(Math.random() * 1e7)
    .toString()
    .replace(/\.\d+/gi, "");

  return (
    <div
      className="HelloWorld"
      title={`You are visitor number ${num}`}
      onMouseOver={onMouseOver}
    >
      <strong>
        {greeting.slice(0, 1).toUpperCase() + greeting.slice(1).toLowerCase()}
      </strong>
      {greeting.endsWith(",") ? (
        " "
      ) : (
        <span style={{ color: "grey" }}>", "</span>
      )}
      <em>{greeted}</em>
      {silent ? "." : "!"}
    </div>
  );
}
@Thom1729
Copy link
Owner

Thom1729 commented Mar 3, 2018

I have updated the commenting behavior to match babel-sublime. (This is what it was supposed to be doing, but there were bugs.) However, this will not address all of the above concerns.

The central problem is that JSX does not support comments. There is no JSX comment syntax at all, and it is unlikely that one will be added. In order to fake it, we use a double-wrapping approach, but that approach has fundamental flaws. In particular, it is impossible for this system to round-trip correctly in the general case. Consider these lines:

<div>
    {foo}
    foo
</div>

If you comment out the first {foo}, you'll get {/*foo*/}. If you comment out the second, you'll get — the same thing? The alternative is transforming the bare JSX text to a string ({/*"foo"*/}, but this is extremely problematic because JSX text behaves differently from JavaScript strings. The translation would be very complicated. And, of course, then commenting foo would produce the same result as commenting {"foo"}. In any case, it would be impossible to reliably reverse the operation by uncommenting, because multiple pre-commented constructs would be mapped onto the same commented construct.

One way to make it work would be to use a special labelled commenting syntax just for JSX text. For example, JSX text foo would transform to {/*JSXText:foo*/}. This could be reliably reversed. But it's an awful, ugly hack, and it does not play nicely with standard JavaScript comments. We'd have to find a transformation that could not possibly collide with real JavaScript, and that's getting harder to guarantee as new JavaScript features are designed. We'd also have to escape JSX text that contains */. It might nevertheless be made to work somehow.

One specific problem that could perhaps be improved is removing the curly brackets when uncommenting. But this runs into round-trip issues as well. Consider the commented {/*foo*/}. Before the commenting, was this foo or {foo}? Whether or not we strip the brackets, there's a good chance we get it wrong. There is one special case in which we could make it work: if the contents inside {/* */} are a single JSX element, then we can safely strip the brackets when uncommenting, and this would probably be a reasonable thing to do under most circumstances.

It's questionable whether the built-in commenting system (inherited from TextMate) can handle something much more complicated than the current system. I've experimented with a system that would hook in to and selectively override the built-in mechanism, but it never reached a favorable usefulness-to-hackiness ratio. There might be a reasonable middle ground using JS Custom's more detailed scoping, but there's an issue with selector scoring that would probably interfere with the nesting typical of JSX code.

I am open to any suggestions you may have. I'm sure that we can improve on babel-sublime's commenting functionality. For now (as of the 1.0.10 release), it should at least match babel-sublime in this regard.

@Thom1729
Copy link
Owner

Thom1729 commented Mar 3, 2018

(See also babel/babel-sublime#262)

@dortamiguel
Copy link
Author

I don't have enough skill to understand how everything works so at the moment I think I can't help to solve this.

@dortamiguel
Copy link
Author

Looks Like vscode handle jsx comments very good, maybe it can be useful for some inspiration.

@Thom1729
Copy link
Owner

Thom1729 commented Mar 3, 2018

It does seem to do a good job overall, though there are some bugs. Integrating it into Sublime's commenting system might be tricky, but I'll look into it.

@Thom1729 Thom1729 changed the title General jsx errors with comments Improve commenting of JSX May 28, 2018
@RiseOoi
Copy link

RiseOoi commented Apr 7, 2019

Will this be of any use?

https://programmingliftoff.com/enable-jsx-commenting-sublime-text-3/

@idr4n
Copy link

idr4n commented May 30, 2019

Also, this package might help as a temporary solution (here).. It is working for me so far...

@Thom1729
Copy link
Owner

Thom1729 commented May 5, 2021

JSX commenting should be significantly improved in v4 for ST4.

@Thom1729 Thom1729 closed this as completed May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants