Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Feature-Request: Add correct highlighting for React.js #11061

Closed
weblogixx opened this issue May 7, 2015 · 45 comments
Closed

Feature-Request: Add correct highlighting for React.js #11061

weblogixx opened this issue May 7, 2015 · 45 comments

Comments

@weblogixx
Copy link

Hi there,

it would be nice if brackets could automatically detect jsx-code in .jsx (and possibly .js) files. This would make it easier to develop React-Components in Brackets.

Please look at the attached screenshot to see what I mean.

brackets

@petetnt
Copy link
Collaborator

petetnt commented May 7, 2015

Have you tried brackets-jsx extension? It should add syntax highlighting for .jsx files.

@weblogixx
Copy link
Author

@petetnt: Yes, I already have it installed, together with jsxHint and other plugins. The code folding and syntax highlighting are looking the way you see them in the screenshot.

The highlighting seems to be related to this plugin (it is a current issue there on automenta/brackets-jsx#2). It seems the plugin itself is deprecated.

@thany
Copy link

thany commented May 11, 2015

So it's kind of understandable, don't you think? The JS you're showing is all kinds of wrong. The fact that React parses it fine, doesn't make it valid JS. To me it's completely normal that Brackets has no idea how to deal with it. In fact, it's doing a pretty good job as it is.

The feature request is not to match the correct bracket, it's to support JSX files in terms of highlighting and such.

@weblogixx
Copy link
Author

@thany: Yes I do understand fully. The problem is that React encourages users to use the .js instead of the older .jsx ending (a thing that seems kind of stupid to me because of all the problem this causes as seen here). As React gains more attention, more projects will be written with it (and files with the .js extension).

@zaggino
Copy link
Contributor

zaggino commented May 12, 2015

I agree with @weblogixx, we should be able to support JSX syntax in .js files.

@petetnt
Copy link
Collaborator

petetnt commented May 12, 2015

CodeMirror doesn't have a React/JS language mode yet either, and I haven't seen attempts at one other than the deprecated code that the brackets-jsx plugin is based on too. See codemirror/codemirror5#3122 for (one) comment from March about adding JSX support for .js files.

@dnbard
Copy link

dnbard commented Jun 3, 2015

Would love to have the JSX support in the Brackets in .js files

bracket jsx wrong ident

The bug with wrong indent is killing me when I'm working with React.

@abdelouahabb
Copy link

@anyone modify you codeMirror file inside your brackets installation so you can get syntax coloration for your nested jsx

replace:

scriptTypes.push({matches: /^(?:text|application)\/(?:x-)?(?:java|ecma)script$|^$/i,

by

scriptTypes.push({matches: /^(?:text|application)\/jsx$|(?:x-)?(?:java|ecma)script$|^$/i,

NB: Still waitng for plugin so we can have calltips :( (Tern-react maybe?)

abdelouahabb/CodeMirror@521c476

@nethip
Copy link
Contributor

nethip commented Jul 6, 2015

@dnbard Did you get a chance to try @abdelouahabb 's advice and see if it makes working with React JS code snippets a little better?

@dnbard
Copy link

dnbard commented Jul 6, 2015

@nethip Nope. This solution didn't worked for me.

@abdelouahabb
Copy link

@dnbard it works here, which file did you changed?

Of course the support is not at 100%, it will be colored as JS . (the markup will not be checked...)
sans titre

@dnbard
Copy link

dnbard commented Jul 6, 2015

@abdelouahabb htmlmixed.js (the only one which has scriptTypes.push({matches: /^(?:text|application)\/(?:x-)?(?:java|ecma)script$|^$/i, string ... )

But I don't need the support of htmlmixed (no one are writing the js code in html file). Support are needed for .js files.

@petetnt
Copy link
Collaborator

petetnt commented Jul 20, 2015

Welp, I started working building proper highlighting for .jsx files today and first obstacle is that .jsx file extension is already defined for .js files.

which of course results in:

Cannot register file extension "jsx" for JSX (React), it already belongs to JavaScript
Menus.js:820 Error setting menu item shortcut: 1

which was added by (issue: #2948 -> PR: #2971). Can you undef a fileExtension via an extension?

@marcelgerber
Copy link
Contributor

Yes you can, using language.removeFileExtension, which would look something like this:

var jsLanguage = LanguageManager.getLanguage("javascript");
jsLanguage.removeFileExtension("js");

@petetnt
Copy link
Collaborator

petetnt commented Jul 25, 2015

Thanks @marcelgerber!

@FezVrasta
Copy link
Contributor

Any news about this feature? I'll probably start a project with React soon and this thing is driving me crazy :(

@lkcampbell
Copy link
Contributor

@petetnt just curious on your approach to a solution.

Are you creating a CodeMirror JSX mode first and then pulling it into Brackets, or some other approach?

@petetnt
Copy link
Collaborator

petetnt commented Nov 14, 2015

@lkcampbell The current idea is to create a javascriptmixed mode for CodeMirror that would also support JSX. See codemirror/codemirror5/issues/3122 and codemirror/codemirror5#3605 (comment) for more info: there's one newer JSX mode already in the wild (that issue 3122) but it's not nearly feature complete.

Sadly I haven't still had much time to work on this and the htmlmixed mode is surprisingly tricky to get right. I'll try to get back to it ASAP.

@lkcampbell
Copy link
Contributor

@petetnt okay let me know if I can help out. I have some free time on my hands recently and it would be really cool to get this functionality finally :).

When you mentioned a mode already in the wild, are you referring to this link:

https://github.com/antimatter15/codemirror-jsx/blob/master/jsx.js

@petetnt
Copy link
Collaborator

petetnt commented Nov 14, 2015

Yup, that's the one. There's also this newer fork that that still has at least some major issues.

If you want to take a crack at it, by all means do. I'd love to see proper support myself too, as pretty much 99% of things I do right now are React based. My version is in a quite bad shape so I don't have much to share with you right now. My current idea was to use overlayMode instead of something similar to the htmlmixed-mode but that idea got pretty much stuck on the complexity of the prop values, that can contain pretty much anything which makes the overlaying very hard. If you want to try that approach, see the Overlay-mode sample

@petetnt
Copy link
Collaborator

petetnt commented Nov 14, 2015

Actually the overlay way isn't that bad way to get started now that I tried it out again:

define(function () {
    var CodeMirror = brackets.getModule("thirdparty/CodeMirror2/lib/codemirror");

    CodeMirror.defineMode("javascriptmixed", function(config, parserConfig) {
        var jsMode = CodeMirror.getMode(config, 'javascript');
        var xmlMode =  CodeMirror.getMode(config, {name: 'xml', htmlMode: true});
        return CodeMirror.overlayMode(jsMode, xmlMode);
    });
});

Result:
image

Like I said, the prop-handling is the real issue

@lkcampbell
Copy link
Contributor

@petetnt, I tweaked your code a tiny bit and used the @marcelgerber suggestion to update the .jsx extension on the fly to jsx (should it be JSX?). Try out this code as an extension and see if it acts how you expect it to act:

https://gist.github.com/lkcampbell/b992ad6926c39331343f

@lkcampbell
Copy link
Contributor

@petetnt also, what theme are you using so I can properly compare colors with you.

I'm still seeing some strange colors associated with the quoted strings in the html tags as well as a few curly braces that are one color for the left bracket and another color for the right. But it is getting closer.

@lkcampbell
Copy link
Contributor

Those strange colors are coming from cm-undefined and cm-error styles. The props are still confusing the parser. It's almost like we need a third customized overlay on top of the Javascript and HTML overlays. Not even sure if this is possible but I will keep investigating.

@petetnt
Copy link
Collaborator

petetnt commented Nov 15, 2015

The theme is Tomorrows Night Eigthties.

For the another overlay, at least something like this is possible:

var fooMode = CodeMirror.defineMode("fooMode", function(config, parserConfig) {
        /** define fooMode **/
        var xmlMode =  CodeMirror.getMode(config, {name: 'xml', htmlMode: true});
        return CodeMirror.overlayMode(fooMode, xmlMode);
});

CodeMirror.defineMode("javascriptmixed", function(config, parserConfig) {
        var jsMode = CodeMirror.getMode(config, 'fooMode');
        return CodeMirror.overlayMode(fooMode, jsMode);
});

I tried this when I started running into the props can be pretty much anything issue and the fooMode (or jsxMode) started to get as complex as a single mode. edit: it might still be worth a try though.

@lkcampbell
Copy link
Contributor

@petetnt, yes I understand the problem better now. I will spend some time with the "triple overlay" approach today to see if there is reasonably simple solution. If not, I will look into the new CodeMirror mode approach.

@lkcampbell
Copy link
Contributor

@petetnt The triple overlay approach does work. I suppose, theoretically, you could keep layering overlays on top of each other until you ran out of memory.

Some questions for you. Purely brainstorming right now.

Is the statement props can be pretty much anything correct? Doesn't the value of the attribute in the HTML portion of the code have to be valid Javascript contained inside of a set of curly braces?

Would it be possible to have a mode that detects a left curly brace and then flips into Javascript mode until hits the matching right curly brace? Then flips back into HTML mode which eventually flips back into Javascript mode again?

@petetnt
Copy link
Collaborator

petetnt commented Nov 15, 2015

@lkcampbell Basically valid JavaScript, but you can pass in JSX-elements (or DOM-elements). (If you are feeling adventurous that is, not advocating anyone to do so though)

Consider something like https://jsfiddle.net/ckeaLv2x/

var Hello = React.createClass({
    render: function() {
        return <div>Hello {this.props.childHello}</div>;
    }
});

var World = React.createClass({
    render: function() {
        var style = {color: "red"};
        return <span style={style}>World, {this.props.name}</span>;
    }
});

ReactDOM.render(
    <Hello childHello={<World name="Pete" />} />,
    document.getElementById('container')
);

So in worst case scenario there needs to be tons of flipflopping around XML and JS mode.

Then there's things such as curly braces in middle of strings:

<Foo bar={"Remember that you shouldn't close to a curly braces inside a string like }-this"} />

But I think the JSMode parser can already correctly catch those, so "Anything valid javascript from "attr={" to the end "}" is a great start. In best case scenario the overlap mode catches the inside xml-elements too.

@lkcampbell
Copy link
Contributor

@petetnt Ugh!!

Okay, send me an email at my same user name at gmail.com.

This clearly is not a weekend project.

Let's discuss more and not use up anymore issue comments on this.

@petetnt
Copy link
Collaborator

petetnt commented Dec 29, 2015

@marijhn just added an JSX mode to CodeMirror @ codemirror/codemirror5@b3f9487 🎆 🚒 🔥

@apla
Copy link

apla commented Dec 29, 2015

@petetnt just packed it to the extension and uploaded: https://github.com/apla/brackets-jsx

So, we can use jsx even earlier than this mode actually appears in brackets.

@abdelouahabb
Copy link

@apla code folding works if you select the block of code,

with selection:
sans titre

without selection
sans titre

@lkcampbell
Copy link
Contributor

@apla You probably already know this, but just in case, issues with the new mode are being tracked at codemirror/codemirror5#3745. The new mode is being updated fairly regularly. That way, you can keep your extension synced up with the changes.

@petetnt or @dabbott, the code folding doesn't appear to be working in Brackets. Do either of you know if the CodeMirror JSX mode and the CodeMirror fold addon are working together?

@petetnt
Copy link
Collaborator

petetnt commented Dec 30, 2015

Pinging @thehogfather as he is the Code Folding expert here. I'd assume that getting the folding working correctly (without selection) might require quite a lot of work.

@lkcampbell
Copy link
Contributor

@petetnt not necessarily a lot of work. The Brackets code folding is built on top of the CodeMirror fold addon code. So, if the new CodeMirror jsx mode works with the existing CodeMirror fold addon code, it should mostly work on the Brackets side as well with minimal tweaking. If it doesn't work, that's a use case that needs to be considered on the CodeMirror side.

@thehogfather
Copy link
Contributor

@petetnt @lkcampbell it should not be a lot of work to get code folding to actually work for jsx. it should be a matter of registering the correct fold helpers for the jsx mode (in this case at least brace fold and xml fold). I'll have a quick look and post any updates here.

@thehogfather
Copy link
Contributor

So it seems that the jsx mode was not correctly returning the contextual mode of the document - always defaulting to jsx (instead of switching between javascript and xml). @apla this pr should fix it. But please test it more than I have - (only tested for about 10 minutes).

Also flagged it up [in this codemirror commit].(codemirror/codemirror5@b3f9487)

@abose
Copy link
Contributor

abose commented Feb 1, 2016

Tracking to verify after cm integration #12177

@ficristo
Copy link
Collaborator

ficristo commented Aug 3, 2016

Now JSX is available.
Closing.

@ficristo ficristo closed this as completed Aug 3, 2016
@timburgess
Copy link

JSX formatting works well for .jsx extension files but there is no mapping to .js at present. Changing languages.json by moving "js" from javascript mode to jsx mode has worked for me to date on regular Javascript files.

@petetnt
Copy link
Collaborator

petetnt commented Mar 26, 2017

@timburgess I've been thinking about using the JSX mode as default for any JavaScript files, the change would be rather simple but haven't just gotten around to do it yet. Basically it's just defaulting to jsx with any js files and monkeypatching LanguageManager to return javascript for any jsx calls to enable code hinting and such.

@timburgess
Copy link

@petetnt problem with moving the extension type .js from javascript to jsx in languages.json is that in the lower right, the filetype will display as 'JSX' not 'Javascript' for simple JS files. There may also be implications in the .brackets.json settings specifying a language of "javascript" not applying.

I think we need to look at the existing "htmlmixed" mode to create something similar for Javascript/JSX.

@petetnt
Copy link
Collaborator

petetnt commented Mar 28, 2017

@timburgess my languages.json now looks like this:

    "javascript": {
        "name": "JavaScript",
        "mode": "jsx",
        "fileExtensions": ["js", "jsx", "js.erb", "jsm", "_js"],
        "blockComment": ["/*", "*/"],
        "lineComment": ["//"]
    },

JSX is already a htmlmixed like mode in the CodeMirror side: it's a mix of HTML mode, JavaScript mode and custom JSX specific changes.

@timburgess
Copy link

@petetnt Nice. That works well.

@keemor
Copy link

keemor commented Jun 28, 2017

If you're using binary version you can achieve this by putting in brackets.json (Debug -> Open Preferences File)

"language.fileExtensions": {
  "js": "jsx"
},

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

No branches or pull requests

17 participants