From 5c9fa5ea18046fbb7e64bef522fd60c31494fff1 Mon Sep 17 00:00:00 2001 From: ning Date: Tue, 29 May 2018 11:48:34 +0800 Subject: [PATCH 1/4] Move registerServiceWorker, notification to utils - utils is a new directory under ./src/ holding misc modules that don't seem to fit anywhere else. - createStore was excluded from utils since the store is a core part of our react/redux app, and so it does make sense as a standalone module in the root ./src/ directory - Also fixed #37 unnecessary yield undefined. --- src/index.tsx | 2 +- src/sagas/index.ts | 4 +--- src/{ => utils}/notification.ts | 0 src/{ => utils}/registerServiceWorker.ts | 0 4 files changed, 2 insertions(+), 4 deletions(-) rename src/{ => utils}/notification.ts (100%) rename src/{ => utils}/registerServiceWorker.ts (100%) diff --git a/src/index.tsx b/src/index.tsx index b9ef940dca..99533363b4 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -9,7 +9,7 @@ import { Store } from 'redux' import ApplicationContainer from './containers/ApplicationContainer' import createStore from './createStore' import { IState } from './reducers/states' -import registerServiceWorker from './registerServiceWorker' +import registerServiceWorker from './utils/registerServiceWorker' import './styles/index.css' diff --git a/src/sagas/index.ts b/src/sagas/index.ts index f6978d45ab..a8b6210b02 100644 --- a/src/sagas/index.ts +++ b/src/sagas/index.ts @@ -6,7 +6,7 @@ import { Context, interrupt, runInContext } from '../slang' import * as actions from '../actions' import * as actionTypes from '../actions/actionTypes' -import { showSuccessMessage, showWarningMessage } from '../notification' +import { showSuccessMessage, showWarningMessage } from '../utils/notification' function* evalCode(code: string, context: Context) { const { result, interrupted } = yield race({ @@ -53,8 +53,6 @@ function* interpreterSaga(): SagaIterator { yield put(actions.clearContext()) yield put(actions.clearReplOutput()) yield call(showSuccessMessage, `Switched to Source \xa7${newChapter}`) - } else { - yield undefined } }) } diff --git a/src/notification.ts b/src/utils/notification.ts similarity index 100% rename from src/notification.ts rename to src/utils/notification.ts diff --git a/src/registerServiceWorker.ts b/src/utils/registerServiceWorker.ts similarity index 100% rename from src/registerServiceWorker.ts rename to src/utils/registerServiceWorker.ts From bda0e637ea1de50ae5720d5eb24d2099039734db Mon Sep 17 00:00:00 2001 From: ning Date: Tue, 29 May 2018 11:58:12 +0800 Subject: [PATCH 2/4] Rename 'genericButton' -> 'controlButton' Also avoided variable name negation by renaming its argument 'notMinimal' -> 'minimal'. Refactor ReplControl to list declarations in decreasing levels of abstraction. (addresses review in PR #35) --- src/components/workspace/ReplControl.tsx | 66 ++++++++++++------------ 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/src/components/workspace/ReplControl.tsx b/src/components/workspace/ReplControl.tsx index 8bce0c3194..be42c1af1c 100644 --- a/src/components/workspace/ReplControl.tsx +++ b/src/components/workspace/ReplControl.tsx @@ -1,6 +1,5 @@ -import * as React from 'react' - import { Button, IconName, Intent } from '@blueprintjs/core' +import * as React from 'react' import { sourceChapters } from '../../reducers/states' @@ -14,48 +13,47 @@ export interface IReplControlProps { handleChapterSelect: (e: React.ChangeEvent) => void } -const chapterSelect = (handleSelect = (e: React.ChangeEvent) => {}) => ( -
- -
-) - class ReplControl extends React.Component { public render() { - const genericButton = ( - label: string, - icon: IconName, - handleClick = () => {}, - intent = Intent.NONE, - notMinimal = false - ) => ( - - ) - const evalButton = genericButton('', 'code', this.props.handleReplEval) - const clearButton = genericButton('', 'remove', this.props.handleReplOutputClear) return (
{chapterSelect(this.props.handleChapterSelect)} - {evalButton} - {clearButton} + {controlButton('', 'code', this.props.handleReplEval)} + {controlButton('', 'remove', this.props.handleReplOutputClear)}
) } } +const controlButton = ( + label: string, + icon: IconName, + handleClick = () => {}, + intent = Intent.NONE, + minimal = true +) => ( + +) + +const chapterSelect = (handleSelect = (e: React.ChangeEvent) => {}) => ( +
+ +
+) + export default ReplControl From fee11081710c5eeeab3917b2866d8a271b1c6294 Mon Sep 17 00:00:00 2001 From: ning Date: Tue, 29 May 2018 12:28:20 +0800 Subject: [PATCH 3/4] Increase test coverage for workspace/Repl.tsx - Add a mockTypeError in mocks for situations that require a mock SourceError of some kind. --- src/components/workspace/__tests__/Repl.tsx | 73 ++++++++++++++++++- .../__tests__/__snapshots__/Repl.tsx.snap | 71 +++++++++++++++++- src/mocks/context.ts | 5 ++ 3 files changed, 142 insertions(+), 7 deletions(-) diff --git a/src/components/workspace/__tests__/Repl.tsx b/src/components/workspace/__tests__/Repl.tsx index 53677487d3..dd215366ab 100644 --- a/src/components/workspace/__tests__/Repl.tsx +++ b/src/components/workspace/__tests__/Repl.tsx @@ -1,12 +1,34 @@ import { shallow } from 'enzyme' import * as React from 'react' -import { ResultOutput } from '../../../reducers/states' +import { mockTypeError } from '../../../mocks/context' import Repl, { Output } from '../Repl' +const mockRunningOutput = { + type: 'running', + consoleLogs: ['a', 'bb', 'cccccccccccccccccccccccccccccccc', 'd'] +} + +const mockCodeOutput = { + type: 'code', + value: "display('');" +} + +const mockResultOutput = { + type: 'result', + value: 42, + consoleLogs: [] +} + +const mockErrorOutput = { + type: 'errors', + errors: [mockTypeError()], + consoleLogs: [] +} + test('Repl renders correctly', () => { const props = { - output: [{ type: 'result', value: 'abc', consoleLogs: [] } as ResultOutput], + output: [mockResultOutput, mockCodeOutput, mockErrorOutput, mockRunningOutput], replValue: '', handleReplValueChange: (newCode: string) => {}, handleReplEval: () => {}, @@ -18,9 +40,52 @@ test('Repl renders correctly', () => { expect(tree.debug()).toMatchSnapshot() }) -test("Output renders correctly for InterpreterOutput.type === 'result'", () => { - const props: ResultOutput = { type: 'result', value: 'def', consoleLogs: [] } +test('Code output renders correctly', () => { + const app = + const tree = shallow(app) + expect(tree.debug()).toMatchSnapshot() +}) + +test('Running output renders correctly', () => { + const app = + const tree = shallow(app) + expect(tree.debug()).toMatchSnapshot() +}) + +test('Result output (no consoleLogs) renders correctly', () => { + const app = + const tree = shallow(app) + expect(tree.debug()).toMatchSnapshot() +}) + +test('Result output (with consoleLogs) renders correctly', () => { + const props = { + ...mockResultOutput, + consoleLogs: mockRunningOutput.consoleLogs + } + const app = + const tree = shallow(app) + expect(tree.debug()).toMatchSnapshot() +}) + +test('Error output (no consoleLogs) renders correctly', () => { + const app = + const tree = shallow(app) + expect(tree.debug()).toMatchSnapshot() +}) + +test('Error output (with consoleLogs) renders correctly', () => { + const props = { + ...mockErrorOutput, + consoleLogs: mockRunningOutput.consoleLogs + } const app = const tree = shallow(app) expect(tree.debug()).toMatchSnapshot() }) + +test('Empty output renders an empty card', () => { + const app = + const tree = shallow(app) + expect(tree.debug()).toMatchSnapshot() +}) diff --git a/src/components/workspace/__tests__/__snapshots__/Repl.tsx.snap b/src/components/workspace/__tests__/__snapshots__/Repl.tsx.snap index 720940b6e3..afb2dbd8ee 100644 --- a/src/components/workspace/__tests__/__snapshots__/Repl.tsx.snap +++ b/src/components/workspace/__tests__/__snapshots__/Repl.tsx.snap @@ -1,9 +1,38 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Output renders correctly for InterpreterOutput.type === 'result' 1`] = ` +exports[`Code output renders correctly 1`] = ` " - 
-    "def"
+  
+    display('');
+  

+"
+`;
+
+exports[`Empty output renders an empty card 1`] = `
+"
+  ''
+"
+`;
+
+exports[`Error output (no consoleLogs) renders correctly 1`] = `
+"
+  
+    Line <unknown>: Expected , got .
+  

+"
+`;
+
+exports[`Error output (with consoleLogs) renders correctly 1`] = `
+"
+  
+    a
+    bb
+    cccccccccccccccccccccccccccccccc
+    d
+  

+  

+  
+    Line <unknown>: Expected , got .
   

 "
 `;
@@ -14,6 +43,9 @@ exports[`Repl renders correctly 1`] = `
     
   
   

+    
+    
+    
     
     

       
@@ -22,3 +54,36 @@ exports[`Repl renders correctly 1`] = `
   

 
"
 `;
+
+exports[`Result output (no consoleLogs) renders correctly 1`] = `
+"
+  
+    42
+  

+"
+`;
+
+exports[`Result output (with consoleLogs) renders correctly 1`] = `
+"
+  
+    a
+    bb
+    cccccccccccccccccccccccccccccccc
+    d
+  

+  
+    42
+  

+"
+`;
+
+exports[`Running output renders correctly 1`] = `
+"
+  
+    a
+    bb
+    cccccccccccccccccccccccccccccccc
+    d
+  

+"
+`;
diff --git a/src/mocks/context.ts b/src/mocks/context.ts
index e64b194ab9..3f5403da90 100644
--- a/src/mocks/context.ts
+++ b/src/mocks/context.ts
@@ -2,6 +2,7 @@ import * as es from 'estree'
 
 import { createContext } from '../slang'
 import { Closure, Context, Frame } from '../slang/types'
+import { TypeError } from '../slang/utils/rttc'
 
 export function mockContext(chapter = 1): Context {
   return createContext(chapter)
@@ -31,3 +32,7 @@ export function mockRuntimeContext(): Context {
 export function mockClosure(): Closure {
   return new Closure({} as es.FunctionExpression, {} as Frame, {} as Context)
 }
+
+export function mockTypeError(): TypeError {
+  return new TypeError({ loc: 0 } as es.Node, '', '', '')
+}

From 36c7991aee1ee178e7d89fe612655f69bd054d14 Mon Sep 17 00:00:00 2001
From: ning 
Date: Wed, 30 May 2018 17:21:07 +0800
Subject: [PATCH 4/4] Abstract controlButton into components/commons

Also fixed typescript type errors on yarn start.
---
 src/components/commons/controlButton.tsx    | 19 +++++++++++++++++
 src/components/commons/index.tsx            |  1 +
 src/components/workspace/Editor.tsx         | 23 +++------------------
 src/components/workspace/ReplControl.tsx    | 19 +----------------
 src/components/workspace/__tests__/Repl.tsx | 17 ++++++++++-----
 src/mocks/context.ts                        |  3 ++-
 6 files changed, 38 insertions(+), 44 deletions(-)
 create mode 100644 src/components/commons/controlButton.tsx
 create mode 100644 src/components/commons/index.tsx

diff --git a/src/components/commons/controlButton.tsx b/src/components/commons/controlButton.tsx
new file mode 100644
index 0000000000..9f2dae378f
--- /dev/null
+++ b/src/components/commons/controlButton.tsx
@@ -0,0 +1,19 @@
+import { Button, IconName, Intent } from '@blueprintjs/core'
+import * as React from 'react'
+
+export const controlButton = (
+  label: string,
+  icon: IconName,
+  handleClick = () => {},
+  intent = Intent.NONE,
+  minimal = true
+) => (
+  
+)
diff --git a/src/components/commons/index.tsx b/src/components/commons/index.tsx
new file mode 100644
index 0000000000..e211007f79
--- /dev/null
+++ b/src/components/commons/index.tsx
@@ -0,0 +1 @@
+export * from './controlButton'
diff --git a/src/components/workspace/Editor.tsx b/src/components/workspace/Editor.tsx
index 993601e964..ae9b8762a4 100644
--- a/src/components/workspace/Editor.tsx
+++ b/src/components/workspace/Editor.tsx
@@ -1,8 +1,7 @@
 import * as React from 'react'
-
 import AceEditor from 'react-ace'
 
-import { Button, IconName, Intent } from '@blueprintjs/core'
+import { controlButton } from '../commons'
 
 import 'brace/mode/javascript'
 import 'brace/theme/cobalt'
@@ -24,27 +23,11 @@ export interface IEditorProps {
 
 class Editor extends React.Component {
   public render() {
-    const genericButton = (
-      label: string,
-      icon: IconName,
-      handleClick = () => {},
-      intent = Intent.NONE,
-      notMinimal = false
-    ) => (
-      
-    )
     const runButton = this.props.isRunning
       ? null
-      : genericButton('', 'play', this.props.handleEditorEval)
+      : controlButton('', 'play', this.props.handleEditorEval)
     const stopButton = this.props.isRunning
-      ? genericButton('', 'stop', this.props.handleInterruptEval)
+      ? controlButton('', 'stop', this.props.handleInterruptEval)
       : null
     return (
       

diff --git a/src/components/workspace/ReplControl.tsx b/src/components/workspace/ReplControl.tsx
index be42c1af1c..16ddef4764 100644
--- a/src/components/workspace/ReplControl.tsx
+++ b/src/components/workspace/ReplControl.tsx
@@ -1,7 +1,7 @@
-import { Button, IconName, Intent } from '@blueprintjs/core'
 import * as React from 'react'
 
 import { sourceChapters } from '../../reducers/states'
+import { controlButton } from '../commons'
 
 /**
  * @property handleEvalEditor  - A callback function for evaluation
@@ -27,23 +27,6 @@ class ReplControl extends React.Component {
   }
 }
 
-const controlButton = (
-  label: string,
-  icon: IconName,
-  handleClick = () => {},
-  intent = Intent.NONE,
-  minimal = true
-) => (
-  
-)
-
 const chapterSelect = (handleSelect = (e: React.ChangeEvent) => {}) => (