highlightjs / highlight.js

JavaScript syntax highlighter with language auto-detection and zero dependencies.
https://highlightjs.org/
BSD 3-Clause "New" or "Revised" License
23.75k stars 3.6k forks source link

jsx - throws exception when variables are referenced on a line that is prior to declaration. #4079

Closed mpjovanovich closed 4 months ago

mpjovanovich commented 4 months ago

jsx code sinppets throw an exception when variables are referenced on a line prior to their declaration.

Which language seems to have the issue?

jsx

Repro Steps

Call function hljs.highlight(language, code) with arguments:

language = 'jsx'
code = '<Text style={bla}>Some text</Text>;\nconst bla = "a";'

Expected behavior

Parses and annotates the code snippet according to current js/jsx error.

Additional context

I'm making presumptions, but test cases show that switching the order of the two code lines will not have a problem. In its submitted state, one of the regex's is coming up blank .

Rationale for Resource Use

I don't know if this was a design choice; perhaps this is as intended. You could make the argument that the coding pattern in the provided snippet is not valid as standalone JSX, so an exception is due. However...

Supporting such patterns for JSX in code snippets reflects much that exists in current codebases, as well as the coding patterns of typical JSX users working in an IDE.

joshgoebel commented 4 months ago

Our TS parser is going to need more context than that. Just throwing HTML on line 1 is not valid TSX. You first need to make it part of an expression, or a return value, etc.

Screen Shot 2024-07-20 at 3 01 11 PM

Also, we don't care about the order of bla, we are not executing your code. We don't care if it even compiles. We do not highlight errors. We just match simple patterns - if your code is mostly syntactically correct we will highlight it.

mpjovanovich commented 4 months ago

I've provided a snippet from a jsx that is not highlighting the way that I'd expect. Your design decisions for scoping, of course, your own. If you don't wish to cover this use case then the issue is closed.

I am well aware that your library was not intended to run code. I did not suggest any expectation that errors should be highlighted.

Given the response to this issue I don't wish to pursue it further. I would appreciate it if you would treat your fellows with a bit more courtesy when responding to issues. We are all quite busy, but lifting each other up is the path forward for us all. I wish the best to your efforts.

joshgoebel commented 4 months ago

I did not suggest any expectation that errors should be highlighted.

Sorry if I mistunderstood - it sounded to me like what you were asking:

"according to current js/jsx error."

...hence my focus on that point. In either case I'd likely have ultimately closed this as a dup of #2998. TLDR: It's hard to get TSX or even JSX correct without a full parser - which we are not. If someone actually had the time to propery tackle this correctly (and also simply) I'd be open to that, but no one has stepped up.

We do allow for other (strong/better) parsers to be easily embedded/integrated... so you could use our highighting engine with someone else's parser - no one has stepped up to try that either - though it would likely result is a much larger JS download size. I think this is the REAL solution for those who insist on super high fidelity high-fidelity highlighting including tiny snippets and edge cases.

const bla = "a";
<Text style={bla}>Some text</Text>;

Is this truly syntactically valid and compilable TSX? If so, it's possible this could be supported post version 12 when we stop caring about auto-detection. Where we wouldn't have to worry about HTML/XML fighting to claim this type of code.

joshgoebel commented 4 months ago

I would appreciate it if you would treat your fellows with a bit more courtesy

I'm just strongly direct. I'm aiming for clear and efficient communication. It helps avoid misunderstandings and respects everyone's time.

mpjovanovich commented 3 months ago

Thanks, yes this looks like #2998. I'll dig into how other syntax highlighting libraries are handling it and see if there's a potential for code port or parser integration.

On Sun, Jul 21, 2024, 1:41 PM Josh Goebel @.***> wrote:

I would appreciate it if you would treat your fellows with a bit more courtesy

I'm just strongly direct. I'm aiming for clear and efficient communication. It helps avoid misunderstandings and respects everyone's time.

— Reply to this email directly, view it on GitHub https://github.com/highlightjs/highlight.js/issues/4079#issuecomment-2241723220, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACTIXCHD24X4V3U2IJL2FL3ZNPXDVAVCNFSM6AAAAABLDY2QTCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBRG4ZDGMRSGA . You are receiving this because you modified the open/close state.Message ID: @.***>