From 5c007e3d53b9914f2ed71abd185434f53a9cd0a4 Mon Sep 17 00:00:00 2001 From: Stuart Taylor Date: Wed, 3 Jan 2018 23:38:52 +0000 Subject: [PATCH] feat(transformers): Disable JS in preview on error --- client/epics/err-epic.js | 4 +-- common/app/routes/Challenges/Preview.jsx | 3 +- .../routes/Challenges/rechallenge/builders.js | 16 ++++++--- .../Challenges/rechallenge/transformers.js | 35 +++++++++++++++++-- .../redux/execute-challenge-epic.js | 9 ++--- common/app/routes/Challenges/redux/index.js | 4 +++ common/app/routes/Challenges/utils/build.js | 10 ++++++ common/utils/polyvinyl.js | 25 ++++++++++--- 8 files changed, 86 insertions(+), 20 deletions(-) diff --git a/client/epics/err-epic.js b/client/epics/err-epic.js index 12c0689843..e94ce68ade 100644 --- a/client/epics/err-epic.js +++ b/client/epics/err-epic.js @@ -4,7 +4,7 @@ export default function errorSaga(actions) { return actions.filter(({ error }) => !!error) .map(({ error }) => error) .doOnNext(error => console.error(error)) - .map(() => makeToast({ - message: 'Something went wrong, please try again later' + .map(error => makeToast({ + message: error.message || 'Something went wrong, please try again later' })); } diff --git a/common/app/routes/Challenges/Preview.jsx b/common/app/routes/Challenges/Preview.jsx index a69b225fc8..0ca2decdbb 100644 --- a/common/app/routes/Challenges/Preview.jsx +++ b/common/app/routes/Challenges/Preview.jsx @@ -1,4 +1,5 @@ -import React, { PropTypes, PureComponent } from 'react'; +import React, { PureComponent } from 'react'; +import PropTypes from 'prop-types'; import { connect } from 'react-redux'; import ns from './ns.json'; diff --git a/common/app/routes/Challenges/rechallenge/builders.js b/common/app/routes/Challenges/rechallenge/builders.js index c38c1a6f30..d5f833953b 100644 --- a/common/app/routes/Challenges/rechallenge/builders.js +++ b/common/app/routes/Challenges/rechallenge/builders.js @@ -50,22 +50,28 @@ const wrapInStyle = partial(transformContents, (content) => ( const setExtToHTML = partial(setExt, 'html'); const padContentWithJsCatch = partial(compileHeadTail, jsCatch); const padContentWithHTMLCatch = partial(compileHeadTail, htmlCatch); +function transformErrorProtect(fn) { + return cond([ + [ matchesProperty('transformError', false), fn ], + [ stubTrue, identity ] + ]); +} -export const jsToHtml = cond([ +export const jsToHtml = transformErrorProtect(cond([ [ matchesProperty('ext', 'js'), flow(padContentWithJsCatch, wrapInScript, setExtToHTML) ], [ stubTrue, identity ] -]); +])); -export const cssToHtml = cond([ +export const cssToHtml = transformErrorProtect(cond([ [ matchesProperty('ext', 'css'), flow(padContentWithHTMLCatch, wrapInStyle, setExtToHTML) ], - [ stubTrue, identity ] -]); + [ stubTrue, identity ] +])); // FileStream::concactHtml( // required: [ ...Object ], diff --git a/common/app/routes/Challenges/rechallenge/transformers.js b/common/app/routes/Challenges/rechallenge/transformers.js index c0b3a9a94a..fb46b3cc6f 100644 --- a/common/app/routes/Challenges/rechallenge/transformers.js +++ b/common/app/routes/Challenges/rechallenge/transformers.js @@ -1,7 +1,9 @@ import { + attempt, cond, flow, identity, + isError, matchesProperty, overEvery, overSome, @@ -105,15 +107,42 @@ export const replaceNBSP = cond([ [ stubTrue, identity ] ]); +const transformErrorWarn = '/* __fcc--TransformError__ */'; + +function tryJSTransform(wrap = identity, defaultAssignment = 'code') { + return function transformWrappedPoly(source) { + const result = attempt(wrap, source); + if (isError(result)) { + const friendlyError = `${result}` + .match(/[\w\W]+?\n/)[0] + .replace(' unknown:', ''); + console.error(friendlyError); + return `var ${defaultAssignment} = null; ${transformErrorWarn}`; + } + return result; + }; +} + +function checkForTransformError(file) { + const potentialError = file.contents.includes(transformErrorWarn); + if (potentialError) { + return { + ...vinyl.setTransformError(true, file) + }; + } + return file; +} + export const babelTransformer = cond([ [ testJS$JSX, flow( partial( vinyl.transformHeadTailAndContents, - babelTransformCode + tryJSTransform(babelTransformCode, 'JSX') ), - partial(vinyl.setExt, 'js') + partial(vinyl.setExt, 'js'), + checkForTransformError ) ], [ stubTrue, identity ] @@ -143,5 +172,5 @@ export function applyTransformers(file, transformers = _transformers) { return obs.flatMap(file => castToObservable(transformer(file))); }, Observable.of(file) - ); + ); } diff --git a/common/app/routes/Challenges/redux/execute-challenge-epic.js b/common/app/routes/Challenges/redux/execute-challenge-epic.js index 10e24f9269..b3ee5a2b3a 100644 --- a/common/app/routes/Challenges/redux/execute-challenge-epic.js +++ b/common/app/routes/Challenges/redux/execute-challenge-epic.js @@ -12,7 +12,8 @@ import { codeLockedSelector, showPreviewSelector, - testsSelector + testsSelector, + disableJSOnError } from './'; import { buildFromFiles, @@ -26,7 +27,8 @@ import { import { createErrorObservable, - challengeSelector + challengeSelector, + doActionOnError } from '../../../redux'; @@ -40,7 +42,6 @@ export function updateMainEpic(actions, { getState }, { document }) { const proxyLogger = new Subject(); const frameMain = createMainFramer(document, getState, proxyLogger); const buildAndFrameMain = actions::ofType( - types.unlockUntrustedCode, types.modernEditorUpdated, types.classicEditorUpdated, types.executeChallenge, @@ -55,7 +56,7 @@ export function updateMainEpic(actions, { getState }, { document }) { .flatMapLatest(() => buildFromFiles(getState(), true) .map(frameMain) .ignoreElements() - .catch(createErrorObservable) + .catch(doActionOnError(() => disableJSOnError())) ); return Observable.merge(buildAndFrameMain, proxyLogger.map(updateOutput)); }); diff --git a/common/app/routes/Challenges/redux/index.js b/common/app/routes/Challenges/redux/index.js index 090de13004..05b34be631 100644 --- a/common/app/routes/Challenges/redux/index.js +++ b/common/app/routes/Challenges/redux/index.js @@ -77,6 +77,7 @@ export const types = createTypes([ 'checkChallenge', createAsyncTypes('submitChallenge'), 'moveToNextChallenge', + 'disableJSOnError', // help 'openHelpModal', @@ -151,6 +152,8 @@ export const submitChallengeComplete = createAction( export const moveToNextChallenge = createAction(types.moveToNextChallenge); +export const disableJSOnError = createAction(types.disableJSOnError); + // help export const openHelpModal = createAction(types.openHelpModal); export const closeHelpModal = createAction(types.closeHelpModal); @@ -306,6 +309,7 @@ export default combineReducers( [ combineActions( types.classicEditorUpdated, + types.disableJSOnError, types.modernEditorUpdated ) ]: state => ({ diff --git a/common/app/routes/Challenges/utils/build.js b/common/app/routes/Challenges/utils/build.js index a1b9d02424..dc3d4455b2 100644 --- a/common/app/routes/Challenges/utils/build.js +++ b/common/app/routes/Challenges/utils/build.js @@ -42,6 +42,15 @@ const globalRequires = [ jQuery ]; +function postTransformCheck(file) { + if (file.transformError) { + // this will enable us to tap into the dipatch pipeline + // and disable JS in the preview + throw new Error('There was an error whilst transforming your code'); + } + return file; +} + export function buildFromFiles(state, shouldProxyConsole) { const files = filesSelector(state); const required = challengeRequiredSelector(state); @@ -49,6 +58,7 @@ export function buildFromFiles(state, shouldProxyConsole) { return createFileStream(files) ::pipe(throwers) ::pipe(applyTransformers) + ::pipe(postTransformCheck) ::pipe(shouldProxyConsole ? proxyLoggerTransformer : identity) ::pipe(jsToHtml) ::pipe(cssToHtml) diff --git a/common/utils/polyvinyl.js b/common/utils/polyvinyl.js index 881b2709be..b0f060d0e5 100644 --- a/common/utils/polyvinyl.js +++ b/common/utils/polyvinyl.js @@ -77,7 +77,8 @@ export function createPoly({ path: name + '.' + ext, key: name + ext, contents, - error: null + error: null, + transformError: false }; } @@ -116,7 +117,7 @@ export function setContent(contents, poly) { }; } -// setExt(contents: String, poly: PolyVinyl) => PolyVinyl +// setExt(ext: String, poly: PolyVinyl) => PolyVinyl export function setExt(ext, poly) { checkPoly(poly); const newPoly = { @@ -129,7 +130,7 @@ export function setExt(ext, poly) { return newPoly; } -// setName(contents: String, poly: PolyVinyl) => PolyVinyl +// setName(name: String, poly: PolyVinyl) => PolyVinyl export function setName(name, poly) { checkPoly(poly); const newPoly = { @@ -142,7 +143,7 @@ export function setName(name, poly) { return newPoly; } -// setError(contents: String, poly: PolyVinyl) => PolyVinyl +// setError(error: Object, poly: PolyVinyl) => PolyVinyl export function setError(error, poly) { invariant( typeof error === 'object', @@ -155,6 +156,19 @@ export function setError(error, poly) { error }; } +// setTransformError(transformError: Boolean, poly: PolyVinyl) => PolyVinyl +export function setTransformError(transformError, poly) { + invariant( + typeof transformError === 'boolean', + 'transformError must be a boolean, but got %', + transformError + ); + checkPoly(poly); + return { + ...poly, + transformError + }; +} // clearHeadTail(poly: PolyVinyl) => PolyVinyl export function clearHeadTail(poly) { @@ -166,6 +180,7 @@ export function clearHeadTail(poly) { }; } +// appendToTail (tail: String, poly: PolyVinyl) => PolyVinyl export function appendToTail(tail, poly) { checkPoly(poly); return { @@ -174,7 +189,7 @@ export function appendToTail(tail, poly) { }; } -// compileHeadTail(contents: String, poly: PolyVinyl) => PolyVinyl +// compileHeadTail(padding: String, poly: PolyVinyl) => PolyVinyl export function compileHeadTail(padding = '', poly) { return clearHeadTail(transformContents( () => [ poly.head, poly.contents, poly.tail ].join(padding),