Skip to content
This repository was archived by the owner on Apr 16, 2026. It is now read-only.

Introduce JSX mode for CodeMirror#3742

Closed
parisk wants to merge 1 commit into
codemirror:masterfrom
sourcelair:jsx-mode
Closed

Introduce JSX mode for CodeMirror#3742
parisk wants to merge 1 commit into
codemirror:masterfrom
sourcelair:jsx-mode

Conversation

@parisk
Copy link
Copy Markdown
Contributor

@parisk parisk commented Dec 27, 2015

This PR introduces a JSX mode for CodeMirror and hopefully closes #3122.

Because of JSX' idiomatic syntax, I preferred implementing this as a completely different mode, than interfering with any of the existing JavaScript or XML modes, despite this was one of @marijnh's proposals at #3122 (#3122 (comment)).

Most of the language features should be supported without any issues.

More info on JSX at:

Comment thread mode/jsx/jsx.js Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to ignore nested tags within a single JSX expression.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it's not. It is defaulting back to JavaScript and re-evaluates if it needs to switch to XML mode (https://github.com/codemirror/CodeMirror/pull/3742/files#diff-8fe55bb9634da263c3fa302cdbefb06cR42).

I could track the dangling open tags and wait for them to close before switching back to JavaScript, but I am not sure if it is a good practice. HTML tags are valid JSX syntax (http://facebook.github.io/react/docs/jsx-in-depth.html#html-tags-vs.-react-components) and from my understanding closing tags is optional in HTML5. I could implement that but sounds like it will introduce a lot of overhead, especially in the state object.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The XML mode already tracks nesting (including understanding HTML close-tag-less tags, but I'm not sure JSX even allows that), and you can use that to determine when you're at the end of a tag.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the officially provided Babel syntax for Sublime does that as well (waits for tags to close before switching back to JavaScript syntax).

Can you give me a hint on how can I get that info from the XML mode state?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Start each JS fragment with its own clean XML mode state, and exit it when after the end of a tag, that state's context property is null (meaning there are no remaining unclosed tags).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Just updated my mode to exit XML mode only when context is null after matching a tag closing token.

@dabbott
Copy link
Copy Markdown
Contributor

dabbott commented Dec 28, 2015

Hi guys. I had an unfortunate mid-air collision with this PR... but I'm putting my PR up as well. It's not 100% finished, but in terms of correctness I think it's very accurate. I originally attempted something like this approach, but found it drastically simpler and less error-prone to add a jsx flag to javascript mode and extend javascript mode directly. I think the closely-coupled-with-js approach will be easier to maintain... but would also understand if you guys don't want to go that route, since it's code that most JS users won't need. Lemme know what you think @marijnh!

I put more notes in the PR.

Have a look here: #3744

@parisk
Copy link
Copy Markdown
Contributor Author

parisk commented Dec 28, 2015

@dabbott great job seems to have been done on your PR! I see that it doesn't apply XML highlighting though on the XML expressions. Do you think that this is something that we could work out? I could try out contributing that on your PR.

@dabbott
Copy link
Copy Markdown
Contributor

dabbott commented Dec 28, 2015

@parisk For sure! Actually, I haven't looked into how to do that stuff at all yet, so that would be great.

Also, bracket matching. And possibly tagname matching. I'm probably styling some stuff with the wrong styles too.

@dabbott
Copy link
Copy Markdown
Contributor

dabbott commented Dec 28, 2015

Can you clarify what you mean by XML highlighting?

This commit introduces a JSX mode for CodeMirror. Supported features:

- XML-extended JavaScript syntax
- JavaScript expressions
- Comments

More info on JSX at:

- https://facebook.github.io/jsx/
- http://facebook.github.io/react/docs/jsx-in-depth.html
@parisk
Copy link
Copy Markdown
Contributor Author

parisk commented Dec 29, 2015

Hi @dabbott I took a look at your first screenshot that does not highlight the tag names, but then found out that you are highlighting them as JavaScript entities after taking a better look at your second one.

After I finish some final touches here, I could try giving some help with what's left.

@dabbott
Copy link
Copy Markdown
Contributor

dabbott commented Dec 29, 2015

Hmmm, yeah, I see what you mean. Logically I think they should be highlighted as JS variables/properties... but at the same time, they give the impression of not being highlighted, since it's relatively uncommon to have locally defined JSX element names or property access in element names.

@petetnt
Copy link
Copy Markdown

petetnt commented Dec 29, 2015

In my opinion @parisk is correct: An JSX mode should

  • Highlight all the known HTML elements
  • Highlight all the custom HTML elements
  • Highlight all declared JSX-elements

Consider something along these lines:

import FooComponent from "./foo";

const BarComponent => (props) => {
  return <div>Bar {props.bar}</div>;
}

class Biz extends React.Component {
  render() {
  return (
    <div classNames="foo">
      <h1>This is a foo component</h1>
      <x-my-component custom-attribute="foo" />
      <FooComponent>
        <BarComponent bar="is Bar" />
        <BazComponent /> 
      </FooComponent>
    </div>
   )
  }
}

In that example:

  1. h1 and div should be highlighted as XML-tags as they are known HTML elements
  2. x-my-component should be highlighted as it's a custom element (see JSX gotchas @ facebook.github.io)
  3. FooComponent and BarComponent should be highlighted as XML-tags as they:
    • Are wrapped in a <
    • Are declared in the file (one as a stateless component, one as imported component)
  4. BazComponent should not be highlighted as it's not declared anywhere

This is a reason why I tried to go with a mixed mode approach initially, similar to http://codemirror.net/mode/htmlmixed/ -mode: this however is a dead end because JSX-attributes can be nested with pretty much anything (JSX-elements, JavaScript, XML...) infinitely. Similarly an overlay mode is a dead end of the same exact reason.

@marijnh
Copy link
Copy Markdown
Member

marijnh commented Dec 29, 2015

HTML allows any tag names, not just the ones in the standard. As such, I don't think highlighting tag names differently is a very accurate or productive approach.

@petetnt
Copy link
Copy Markdown

petetnt commented Dec 29, 2015

@marijnh Actually you are correct, I remembered that React would throw went it didn't understand the element but this doesn't seem to be the case, at least anymore.

@marijnh marijnh closed this in b3f9487 Dec 29, 2015
@marijnh
Copy link
Copy Markdown
Member

marijnh commented Dec 29, 2015

Well, I suppose if three people are working on one, the need for this mode is pressing. And it looks like it's going to be less work to write one myself rather than to keep hinting and reviewing and pushing people in the right direction. You can find my version in b3f9487. Please test it and submit failing test cases. If this helps you in a financial way, a donation to cover the morning spent on this is much appreciated.

@parisk
Copy link
Copy Markdown
Contributor Author

parisk commented Dec 29, 2015

🎉 really happy to see this implemented at all. I hope that our contributions helped you implement it faster and better.

@dabbott
Copy link
Copy Markdown
Contributor

dabbott commented Dec 29, 2015

Great! Glad to have this in. We sent you an email re: donation and buying a support plan.

@leeoniya
Copy link
Copy Markdown
Contributor

http://codemirror.net/mode/jsx/index.html

some self closed tags /> show up red (errors?)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants