TypescriptReact (.tsx) commenting

[curiousercreative]

Have you checked for existing feature requests?

• Completed

Summary

Commenting out JSX elements should be done with {/* commented out code here */} rather than // commented out code here. The language-babel package does this correctly for reference.

What benefits does this feature provide?

Better DX for commenting out blocks of tsx (jsx in a typescript file)

Any alternatives?

You can map all .tsx files to language-babel

Other examples:

No response

[confused-Techie]

Sorry this issue never got a comment originally.

I did intend to respond after doing some initial research, but must have gotten sidetracked.

But if memory serves, my initial research on this issue found the reason that community package has better comments, is because it determines the comment within JavaScript, rather than entirely as a grammar file. Which could be possible but would require a far bit of work. But their implementation is rather simple and something we could absolutely learn from to include within the core, and would be more than happy to see a PR addressing this.

But who knows, maybe this is something that would be much more possible after we merge the experimental Tree-Sitter updates

[savetheclocktower]

I didn't notice this either. The fix is really rather easy and would just require a scope-specific setting. I might submit a fix tomorrow if I get some time.

[confused-Techie]

@savetheclocktower That would be amazing if you could!

Especially if the problem is truly trivial to resolve I'd be glad to hear!

[savetheclocktower]

OK, needless to say, it's more complex than I predicted, but it's a good case study.

We don't have to compare this to the language-babel grammar; we can just compare it to the built-in TextMate-style TypeScriptReact grammar. The language-typescript package defines this in settings/TypeScriptReact.cson:

'.source.tsx':
  'editor':
    'commentStart': '// '

'.meta.tag.tsx':
  'editor':
    'commentStart': '{/* ',
    'commentEnd': ' */}',

This covers the scenario — admittedly a bit unusual — where the comment delimiters within a single grammar vary based on where we are in the file. I can’t think of another situation where this is true — except for situations that are obviously about one language being embedded in another, like JavaScript inside a SCRIPT block in HTML.

When Atom added tree-sitter grammars, they apparently thought that comment delimiter metadata was better defined in the grammar file itself, as seen in the tree-sitter-tsx.cson file:

comments:
  start: '// '

And they thought that they’d be able to figure out the right comment delimiters for a given range by doing this lookup:

function commentStringsForPosition(position) {
  const range =
    this.firstNonWhitespaceRange(position.row) ||
    new Range(position, position);
  const { grammar } = this.getSyntaxNodeAndGrammarContainingRange(range);
  return grammar.commentStrings;
}

But this can’t handle our unusual scenario where we might need // on one line and {/* */} on the very next without the grammar changing. If JSX had its own grammar and were handled via an injection, this would work fine, but this just underscores how brittle the system is.

If I rewrote this code to do what the TextMate grammars do — i.e., a scope-specific setting lookup — it’d probably have a lot of unintended consequences, and it doesn’t make much sense for me to do something this disruptive for a system that we’re going to deprecate in a matter of months.

@curiousercreative, the best workaround for now is to use either the language-babel grammar or the TextMate-style TypeScriptReact grammar (go to the grammar-selector package’s setting, uncheck “Hide Duplicate TextMate grammars,” and then you’ll see it as an option in the grammar selector; it’ll be the “TypeScriptReact” option without a badge next to it).

But this just confirms my hunch that the old way was better. The commentStringsForPosition function in the modern-tree-sitter mode will first try a scoped-setting retrieval, then will fall back to a grammar-based lookup only if it doesn’t find anything. This ticket will be fixed whenever we ship modern-tree-sitter without an experimental flag.

[confused-Techie]

@savetheclocktower I appreciate you looking into this, and coming to a definitive decision, thanks a ton for your efforts!

[savetheclocktower]

@curiousercreative I've finally addressed this in #677. It'll go out in version 1.109 in about a month. Quite sorry it took so long.

Edited to add: This fix will be present on the modern Tree-sitter TSX grammar, so it'll require that you enable the Use Modern Tree-Sitter Implementation in the core settings.