Created attachment 411124 [details] [Image] Bug We started supporting JSX in 2018 (bug 181607). We rely on CodeMirror for syntax highlighting, perhaps we should update the JSX mode (External/CodeMirror/jsx.js).
<rdar://problem/70210975>
I updated External/CodeMirror/jsx.js and it didn't solve the problem. In fact, the JSX mode of the current stable CodeMirror (5.58.1) is nearly identical to the one we use. The problem is that https://reactjs.org/docs/create-a-new-react-app.html has JSX in App.js. Note the "js" extension, not "jsx". It's served with MIME-type text/javascript, too. Web Inspector simply doesn't use JSX mode because it thinks it's a plain JS.
Chrome DevTools handles JSX well and it also uses CodeMirror. Chrome seems to use JSX mode for all JS resources. This feels wrong but perhaps this is a lesser evil.
Created attachment 417413 [details] Patch
Created attachment 417414 [details] [Image] With patch applied
Comment on attachment 417413 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417413&action=review r-, see below > Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:691 > + CodeMirror.defineMIME(WI.mimeTypeForFileExtension("js"), "jsx"); I think CodeMirror only allows one mode per MIME type, which means that this would cause every `"text/javascript"` file to now be interpreted as JSX. IMO, if the extension is `.js` and the MIME type is `text/javascript` then we really should be treating it as JavaScript, not JSX.
Comment on attachment 417413 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417413&action=review >> Source/WebInspectorUI/UserInterface/Views/CodeMirrorAdditions.js:691 >> + CodeMirror.defineMIME(WI.mimeTypeForFileExtension("js"), "jsx"); > > I think CodeMirror only allows one mode per MIME type, which means that this would cause every `"text/javascript"` file to now be interpreted as JSX. IMO, if the extension is `.js` and the MIME type is `text/javascript` then we really should be treating it as JavaScript, not JSX. This change shouldn't affect syntax highlighting of syntactically valid JS. Besides, I don't know any better way of resolving this.
Created attachment 417631 [details] Patch I changed the patch to only use JSX mode for source map JS resources. Their MIME type is "synthetic"; it's determined from the resource extension already.
Comment on attachment 417631 [details] Patch I have no objections to this approach, with the precondition that it be tested on vanilla JS files to ensure it's not a pretty-printing regression. What testing has been performed with this change?
Can you verify that whether this interferes with pretty printing or other source map logic? More generally though, I don't think this is a good path forward for a few reasons: - it's preferential towards react - it's incorrect (JSX !== JavaScript) - JSX is nonstandard and may clash in unexpected ways with future changes to JS I'd be much less concerned with this being a default-off setting like "treat JS resources as JSX" or something, but even then I'm not sure this is tested/known enough not to cause issues with logic throughout Web Inspector.
Devin, do you have a concrete example of a well formatted JS file that fails when using JSX formatting?
Comment on attachment 417631 [details] Patch r=me We need to move on, I don't think this is going to make pretty-printing source-mapped files any worse.
Committed r274230: <https://commits.webkit.org/r274230> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417631 [details].