From a1877f4b8464d116d8e325931a077c51275316a3 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) # Conflicts: # src/actions/sources/loadSourceText.js # src/actions/tests/ast.spec.js # src/components/Editor/index.js --- src/actions/ast.js | 27 +++++++++++++++++- 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 | 27 +++++++++++++----- 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 ++- 13 files changed, 156 insertions(+), 25 deletions(-) create mode 100644 src/actions/tests/fixtures/reactComponent.js diff --git a/src/actions/ast.js b/src/actions/ast.js index 5d1f37a9be..3e36302ee0 100644 --- a/src/actions/ast.js +++ b/src/actions/ast.js @@ -14,7 +14,8 @@ import { PROMISE } from "../utils/redux/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); @@ -50,6 +74,7 @@ export function setSymbols(sourceId: SourceId) { }); dispatch(setEmptyLines(source.id)); + dispatch(setSourceMetaData(source.id)); }; } diff --git a/src/actions/tests/ast.spec.js b/src/actions/tests/ast.spec.js index b7920934d1..5b0145d452 100644 --- a/src/actions/tests/ast.spec.js +++ b/src/actions/tests/ast.spec.js @@ -9,7 +9,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 = { @@ -34,7 +39,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 = { @@ -61,6 +67,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", () => { 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 06c3412a5e..5cb44f2317 100644 --- a/src/components/Editor/index.js +++ b/src/components/Editor/index.js @@ -20,7 +20,8 @@ import { getCoverageEnabled, getConditionalPanelLine, getFileSearchModifiers, - getFileSearchQuery + getFileSearchQuery, + getSourceMetaData, } from "../../selectors"; import actions from "../../actions"; @@ -36,6 +37,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, @@ -81,10 +83,17 @@ type Props = { startPanelSize: number, endPanelSize: number, conditionalPanelLine: number, - openConditionalPanel: Function, - closeConditionalPanel: Function, - continueToHere: Function, - setContextMenu: Function + sourceMetaData: SourceMetaDataType, + + // Actions + openConditionalPanel: number => void, + closeConditionalPanel: void => void, + setContextMenu: (string, any) => void, + continueToHere: number => void, + toggleBreakpoint: number => void, + addOrToggleDisabledBreakpoint: number => void, + jumpToMappedLocation: any => void, + traverseResults: (boolean, Object) => void }; type State = { @@ -459,7 +468,7 @@ class Editor extends PureComponent { } setText(props) { - const { selectedSource } = props; + const { selectedSource, sourceMetaData } = props; if (!this.state.editor) { return; } @@ -477,7 +486,11 @@ class Editor extends PureComponent { } if (selectedSource) { - return showSourceText(this.state.editor, selectedSource.toJS()); + return showSourceText( + this.state.editor, + selectedSource.toJS(), + sourceMetaData + ); } } 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 97ee400bc7..a4c1ec5cd3 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; @@ -199,13 +200,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 c6c69c532d..73126205d0 100644 --- a/src/utils/tests/source.spec.js +++ b/src/utils/tests/source.spec.js @@ -119,7 +119,7 @@ describe("sources", () => { expect(getMode(source)).toBe("elm"); }); - it("jsx", () => { + it("returns jsx if contentType jsx is given", () => { const source = { contentType: "text/jsx", text: "

", @@ -128,6 +128,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 });