From 105b9ea1b3cbd11f71a9d1680a24d40a1d20f56b Mon Sep 17 00:00:00 2001 From: Martin Schmid Date: Mon, 6 Nov 2017 17:02:38 +0100 Subject: [PATCH] Improve JSX syntax highlighting (#4539) --- src/actions/ast.js | 26 ++++++++++++++++- src/actions/sources/loadSourceText.js | 3 +- src/actions/tests/ast.spec.js | 30 ++++++++++++++++++-- src/actions/tests/fixtures/reactComponent.js | 7 +++++ src/components/Editor/Breakpoint.js | 7 +++-- src/components/Editor/Breakpoints.js | 11 ++++--- src/components/Editor/index.js | 14 +++++++-- src/reducers/ast.js | 23 +++++++++++++-- src/utils/editor/source-documents.js | 11 +++++-- src/utils/source.js | 10 ++++++- src/utils/tests/source.spec.js | 18 +++++++++++- src/workers/parser/frameworks.js | 5 +++- src/workers/parser/index.js | 1 + src/workers/parser/worker.js | 4 ++- 14 files changed, 148 insertions(+), 22 deletions(-) create mode 100644 src/actions/tests/fixtures/reactComponent.js diff --git a/src/actions/ast.js b/src/actions/ast.js index 1eefc7fa9d..acbb16b4c0 100644 --- a/src/actions/ast.js +++ b/src/actions/ast.js @@ -14,7 +14,8 @@ import { PROMISE } from "./utils/middleware/promise"; import { getSymbols, getEmptyLines, - getOutOfScopeLocations + getOutOfScopeLocations, + isReactComponent } from "../workers/parser"; import { findBestMatchExpression } from "../utils/ast"; @@ -29,6 +30,29 @@ const extraProps = { react: { displayName: "this._reactInternalInstance.getName()" } }; +export function setSourceMetaData(sourceId: SourceId) { + return async ({ dispatch, getState }: ThunkArgs) => { + const sourceRecord = getSource(getState(), sourceId); + if (!sourceRecord) { + return; + } + + const source = sourceRecord.toJS(); + if (!source.text || source.isWasm) { + return; + } + + const isReactComp = await isReactComponent(source); + dispatch({ + type: "SET_SOURCE_METADATA", + sourceId: source.id, + sourceMetaData: { + isReactComponent: isReactComp + } + }); + }; +} + export function setSymbols(sourceId: SourceId) { return async ({ dispatch, getState }: ThunkArgs) => { const sourceRecord = getSource(getState(), sourceId); diff --git a/src/actions/sources/loadSourceText.js b/src/actions/sources/loadSourceText.js index ec893256a5..90f09e8eeb 100644 --- a/src/actions/sources/loadSourceText.js +++ b/src/actions/sources/loadSourceText.js @@ -1,6 +1,6 @@ // @flow import { PROMISE } from "../utils/middleware/promise"; -import { setEmptyLines, setSymbols } from "../ast"; +import { setEmptyLines, setSymbols, setSourceMetaData } from "../ast"; import { getSource } from "../../selectors"; import { setSource } from "../../workers/parser"; import type { Source } from "../../types"; @@ -45,5 +45,6 @@ export function loadSourceText(source: Source) { await setSource(newSource); await dispatch(setSymbols(source.id)); await dispatch(setEmptyLines(source.id)); + await dispatch(setSourceMetaData(source.id)); }; } diff --git a/src/actions/tests/ast.spec.js b/src/actions/tests/ast.spec.js index 4f8139db79..4306848ea8 100644 --- a/src/actions/tests/ast.spec.js +++ b/src/actions/tests/ast.spec.js @@ -6,7 +6,12 @@ import { } from "../../utils/test-head"; import readFixture from "./helpers/readFixture"; -const { getSymbols, getEmptyLines, getOutOfScopeLocations } = selectors; +const { + getSymbols, + getEmptyLines, + getOutOfScopeLocations, + getSourceMetaData +} = selectors; import getInScopeLines from "../../selectors/linesInScope"; const threadClient = { @@ -31,7 +36,8 @@ const threadClient = { const sourceTexts = { "base.js": "function base(boo) {}", "foo.js": "function base(boo) { return this.bazz; } outOfScope", - "scopes.js": readFixture("scopes.js") + "scopes.js": readFixture("scopes.js"), + "reactComponent.js": readFixture("reactComponent.js") }; const evaluationResult = { @@ -51,6 +57,26 @@ describe("ast", () => { expect(emptyLines).toMatchSnapshot(); }); }); + describe("setSourceMetaData", () => { + it("should detect react components", async () => { + const { dispatch, getState } = createStore(threadClient); + const source = makeSource("reactComponent.js"); + await dispatch(actions.newSource(source)); + await dispatch(actions.loadSourceText({ id: "reactComponent.js" })); + + const sourceMetaData = getSourceMetaData(getState(), source.id); + expect(sourceMetaData).toEqual({ isReactComponent: true }); + }); + it("should not give false positive on non react components", async () => { + const { dispatch, getState } = createStore(threadClient); + const source = makeSource("base.js"); + await dispatch(actions.newSource(source)); + await dispatch(actions.loadSourceText({ id: "base.js" })); + + const sourceMetaData = getSourceMetaData(getState(), source.id); + expect(sourceMetaData).toEqual({ isReactComponent: false }); + }); + }); describe("setSymbols", () => { describe("when the source is loaded", () => { it("should be able to set symbols", async () => { diff --git a/src/actions/tests/fixtures/reactComponent.js b/src/actions/tests/fixtures/reactComponent.js new file mode 100644 index 0000000000..526c852d99 --- /dev/null +++ b/src/actions/tests/fixtures/reactComponent.js @@ -0,0 +1,7 @@ +import React, { Component } from "react"; + +class FixtureComponent extends Component { + render() { + return null; + } +} diff --git a/src/components/Editor/Breakpoint.js b/src/components/Editor/Breakpoint.js index f2e4f9bf19..b8c69ee5d1 100644 --- a/src/components/Editor/Breakpoint.js +++ b/src/components/Editor/Breakpoint.js @@ -24,7 +24,8 @@ function makeMarker(isDisabled: boolean) { type Props = { breakpoint: Object, selectedSource: Object, - editor: Object + editor: Object, + sourceMetaData: Object }; class Breakpoint extends Component { @@ -36,7 +37,7 @@ class Breakpoint extends Component { } addBreakpoint() { - const { breakpoint, editor, selectedSource } = this.props; + const { breakpoint, editor, selectedSource, sourceMetaData } = this.props; // Hidden Breakpoints are never rendered on the client if (breakpoint.hidden) { @@ -52,7 +53,7 @@ class Breakpoint extends Component { const sourceId = selectedSource.get("id"); const line = toEditorLine(sourceId, breakpoint.location.line); - showSourceText(editor, selectedSource.toJS()); + showSourceText(editor, selectedSource.toJS(), sourceMetaData); editor.codeMirror.setGutterMarker( line, diff --git a/src/components/Editor/Breakpoints.js b/src/components/Editor/Breakpoints.js index 030e632358..b99136e19c 100644 --- a/src/components/Editor/Breakpoints.js +++ b/src/components/Editor/Breakpoints.js @@ -6,7 +6,7 @@ import React, { Component } from "react"; import Breakpoint from "./Breakpoint"; import actions from "../../actions"; -import { getSelectedSource } from "../../selectors"; +import { getSelectedSource, getSourceMetaData } from "../../selectors"; import getVisibleBreakpoints from "../../selectors/visibleBreakpoints"; import { makeLocationId } from "../../utils/breakpoint"; import { isLoaded } from "../../utils/source"; @@ -16,7 +16,8 @@ import type { SourceRecord, BreakpointsMap } from "../../reducers/types"; type Props = { selectedSource: SourceRecord, breakpoints: BreakpointsMap, - editor: Object + editor: Object, + sourceMetaData: Object }; class Breakpoints extends Component { @@ -32,7 +33,7 @@ class Breakpoints extends Component { } render() { - const { breakpoints, selectedSource, editor } = this.props; + const { breakpoints, selectedSource, editor, sourceMetaData } = this.props; if (!selectedSource || !breakpoints || selectedSource.get("isBlackBoxed")) { return null; @@ -46,6 +47,7 @@ class Breakpoints extends Component { key={makeLocationId(bp.location)} breakpoint={bp} selectedSource={selectedSource} + sourceMetaData={sourceMetaData} editor={editor} /> ); @@ -58,7 +60,8 @@ class Breakpoints extends Component { export default connect( state => ({ breakpoints: getVisibleBreakpoints(state), - selectedSource: getSelectedSource(state) + selectedSource: getSelectedSource(state), + sourceMetaData: getSourceMetaData(state, getSelectedSource(state).id) }), dispatch => bindActionCreators(actions, dispatch) )(Breakpoints); diff --git a/src/components/Editor/index.js b/src/components/Editor/index.js index 789aff0ebf..b7bec5a062 100644 --- a/src/components/Editor/index.js +++ b/src/components/Editor/index.js @@ -16,6 +16,7 @@ import { getSelectedSource, getHitCountForSource, getCoverageEnabled, + getSourceMetaData, getConditionalPanelLine } from "../../selectors"; @@ -35,6 +36,7 @@ import EmptyLines from "./EmptyLines"; import GutterMenu from "./GutterMenu"; import EditorMenu from "./EditorMenu"; import ConditionalPanel from "./ConditionalPanel"; +import type { SourceMetaDataType } from "../../reducers/ast"; import { showSourceText, @@ -74,6 +76,7 @@ type Props = { startPanelSize: number, endPanelSize: number, conditionalPanelLine: number, + sourceMetaData: SourceMetaDataType, // Actions openConditionalPanel: number => void, @@ -463,7 +466,7 @@ class Editor extends PureComponent { } setText(props) { - const { selectedSource } = props; + const { selectedSource, sourceMetaData } = props; if (!this.state.editor) { return; } @@ -481,7 +484,11 @@ class Editor extends PureComponent { } if (selectedSource) { - return showSourceText(this.state.editor, selectedSource.toJS()); + return showSourceText( + this.state.editor, + selectedSource.toJS(), + sourceMetaData + ); } } @@ -607,7 +614,8 @@ const mapStateToProps = state => { hitCount: getHitCountForSource(state, sourceId), selectedFrame: getSelectedFrame(state), coverageOn: getCoverageEnabled(state), - conditionalPanelLine: getConditionalPanelLine(state) + conditionalPanelLine: getConditionalPanelLine(state), + sourceMetaData: getSourceMetaData(state, sourceId) }; }; diff --git a/src/reducers/ast.js b/src/reducers/ast.js index b3c1706e02..a220d1d612 100644 --- a/src/reducers/ast.js +++ b/src/reducers/ast.js @@ -22,6 +22,12 @@ type EmptyLinesType = number[]; export type SymbolsMap = Map; export type EmptyLinesMap = Map; +export type SourceMetaDataType = { + isReactComponent: boolean +}; + +export type SourceMetaDataMap = Map; + export type Preview = | {| updating: true |} | null @@ -38,7 +44,8 @@ export type ASTState = { symbols: SymbolsMap, emptyLines: EmptyLinesMap, outOfScopeLocations: ?Array, - preview: Preview + preview: Preview, + sourceMetaData: SourceMetaDataMap }; export function initialState() { @@ -47,7 +54,8 @@ export function initialState() { symbols: I.Map(), emptyLines: I.Map(), outOfScopeLocations: null, - preview: null + preview: null, + sourceMetaData: I.Map() }: ASTState) )(); } @@ -111,6 +119,13 @@ function update( return initialState(); } + case "SET_SOURCE_METADATA": { + return state.setIn( + ["sourceMetaData", action.sourceId], + action.sourceMetaData + ); + } + default: { return state; } @@ -167,4 +182,8 @@ export function getPreview(state: OuterState) { return state.ast.get("preview"); } +export function getSourceMetaData(state: OuterState, sourceId: string) { + return state.ast.getIn(["sourceMetaData", sourceId]) || {}; +} + export default update; diff --git a/src/utils/editor/source-documents.js b/src/utils/editor/source-documents.js index 2928694d7a..ffafe0c357 100644 --- a/src/utils/editor/source-documents.js +++ b/src/utils/editor/source-documents.js @@ -5,6 +5,7 @@ import { getMode } from "../source"; import type { Source } from "debugger-html"; import { isWasm, getWasmLineNumberFormatter, renderWasmText } from "../wasm"; import { resizeBreakpointGutter, resizeToggleButton } from "../ui"; +import type { SourceMetaDataType } from "../../reducers/ast"; let sourceDocs = {}; @@ -81,19 +82,25 @@ function setEditorText(editor: Object, source: Source) { * Handle getting the source document or creating a new * document with the correct mode and text. */ -function showSourceText(editor: Object, source: Source) { +function showSourceText( + editor: Object, + source: Source, + sourceMetaData: SourceMetaDataType +) { if (!source) { return; } let doc = getDocument(source.id); if (editor.codeMirror.doc === doc) { + editor.setMode(getMode(source, sourceMetaData)); return; } if (doc) { editor.replaceDocument(doc); updateLineNumberFormat(editor, source.id); + editor.setMode(getMode(source, sourceMetaData)); return doc; } @@ -102,7 +109,7 @@ function showSourceText(editor: Object, source: Source) { editor.replaceDocument(doc); setEditorText(editor, source); - editor.setMode(getMode(source)); + editor.setMode(getMode(source, sourceMetaData)); updateLineNumberFormat(editor, source.id); } diff --git a/src/utils/source.js b/src/utils/source.js index f82f80e0ce..9871baeba4 100644 --- a/src/utils/source.js +++ b/src/utils/source.js @@ -11,6 +11,7 @@ import { basename } from "../utils/path"; import { parse as parseURL } from "url"; import type { Source } from "../types"; +import type { SourceMetaDataType } from "../reducers/ast"; type transformUrlCallback = string => string; @@ -204,13 +205,20 @@ function getSourceLineCount(source: Source) { * @static */ -function getMode(source: Source) { +function getMode(source: Source, sourceMetaData: SourceMetaDataType) { const { contentType, text, isWasm, url } = source; if (!text || isWasm) { return { name: "text" }; } + if ( + (url && url.match(/\.jsx$/i)) || + (sourceMetaData && sourceMetaData.isReactComponent) + ) { + return "jsx"; + } + // if the url ends with .marko we set the name to Javascript so // syntax highlighting works for marko too if (url && url.match(/\.marko$/i)) { diff --git a/src/utils/tests/source.spec.js b/src/utils/tests/source.spec.js index ef7fcab428..e25b911242 100644 --- a/src/utils/tests/source.spec.js +++ b/src/utils/tests/source.spec.js @@ -127,7 +127,7 @@ describe("sources", () => { expect(getMode(source)).toBe("elm"); }); - it("jsx", () => { + it("returns jsx if contentType jsx is given", () => { const source = { contentType: "text/jsx", text: "

", @@ -136,6 +136,22 @@ describe("sources", () => { expect(getMode(source)).toBe("jsx"); }); + it("returns jsx if sourceMetaData says it's a react component", () => { + const source = { + text: "

", + url: "" + }; + expect(getMode(source, { isReactComponent: true })).toBe("jsx"); + }); + + it("returns jsx if the fileExtension is .jsx", () => { + const source = { + text: "

", + url: "myComponent.jsx" + }; + expect(getMode(source)).toBe("jsx"); + }); + it("typescript", () => { const source = { contentType: "text/typescript", diff --git a/src/workers/parser/frameworks.js b/src/workers/parser/frameworks.js index c0f69b9de4..0bbad0334b 100644 --- a/src/workers/parser/frameworks.js +++ b/src/workers/parser/frameworks.js @@ -21,7 +21,10 @@ function importsReact(imports) { function extendsComponent(classes) { let result = false; classes.some(classObj => { - if (classObj.parent.name === "Component") { + if ( + classObj.parent.name === "Component" || + classObj.parent.name === "PureComponent" + ) { result = true; } }); diff --git a/src/workers/parser/index.js b/src/workers/parser/index.js index eedee86084..27262a5466 100644 --- a/src/workers/parser/index.js +++ b/src/workers/parser/index.js @@ -21,6 +21,7 @@ export const hasSource = dispatcher.task("hasSource"); export const setSource = dispatcher.task("setSource"); export const clearSources = dispatcher.task("clearSources"); export const hasSyntaxError = dispatcher.task("hasSyntaxError"); +export const isReactComponent = dispatcher.task("isReactComponent"); export type { SymbolDeclaration, SymbolDeclarations } from "./getSymbols"; export type { AstLocation } from "./types"; diff --git a/src/workers/parser/worker.js b/src/workers/parser/worker.js index b870be83b8..5923e20391 100644 --- a/src/workers/parser/worker.js +++ b/src/workers/parser/worker.js @@ -8,6 +8,7 @@ import getOutOfScopeLocations from "./getOutOfScopeLocations"; import { getNextStep } from "./steps"; import getEmptyLines from "./getEmptyLines"; import { hasSyntaxError } from "./validate"; +import { isReactComponent } from "./frameworks"; import { workerUtils } from "devtools-utils"; const { workerHandler } = workerUtils; @@ -26,5 +27,6 @@ self.onmessage = workerHandler({ getVariablesInScope, getNextStep, getEmptyLines, - hasSyntaxError + hasSyntaxError, + isReactComponent });