From a2864421361062f91b8102dc1bd3149ca562555f Mon Sep 17 00:00:00 2001 From: Ives van Hoorne Date: Sun, 9 Jul 2017 23:39:23 +0200 Subject: [PATCH 1/2] Support for cyclic dependencies --- src/sandbox/eval/index.js | 56 +++++++++++++++++++++++++---- src/sandbox/eval/js/babel-parser.js | 2 +- src/sandbox/eval/js/index.js | 29 +++++++++++++-- 3 files changed, 77 insertions(+), 10 deletions(-) diff --git a/src/sandbox/eval/index.js b/src/sandbox/eval/index.js index 1817c4f9056..dcfc21b4ec4 100644 --- a/src/sandbox/eval/index.js +++ b/src/sandbox/eval/index.js @@ -14,6 +14,7 @@ function doEval( directories: Array, externals: Object, depth: ?number, + parentModule: ?Module, ) { const html = /\.html$/; const css = /\.css$/; @@ -21,22 +22,57 @@ function doEval( const js = /\.js$/; if (html.test(mainModule.title)) { - return evalRaw(mainModule, modules, directories, externals, depth); + return evalRaw( + mainModule, + modules, + directories, + externals, + depth, + parentModule, + ); } if (css.test(mainModule.title)) { - return evalCSS(mainModule, modules, directories, externals, depth); + return evalCSS( + mainModule, + modules, + directories, + externals, + depth, + parentModule, + ); } if (json.test(mainModule.title) || mainModule.title === '.babelrc') { - return evalJson(mainModule, modules, directories, externals, depth); + return evalJson( + mainModule, + modules, + directories, + externals, + depth, + parentModule, + ); } if (js.test(mainModule.title)) { - return evalJS(mainModule, modules, directories, externals, depth); + return evalJS( + mainModule, + modules, + directories, + externals, + depth, + parentModule, + ); } - return evalRaw(mainModule, modules, directories, externals, depth); + return evalRaw( + mainModule, + modules, + directories, + externals, + depth, + parentModule, + ); } export function deleteCache(module: Module) { @@ -49,6 +85,7 @@ const evalModule = ( directories: Array, externals: Object, depth: number = 0, + parentModule: Array = [], ) => { if (depth > MAX_DEPTH) { throw new Error( @@ -56,7 +93,14 @@ const evalModule = ( ); } try { - return doEval(mainModule, modules, directories, externals, depth); + return doEval( + mainModule, + modules, + directories, + externals, + depth, + parentModule, + ); } catch (e) { e.module = e.module || mainModule; throw e; diff --git a/src/sandbox/eval/js/babel-parser.js b/src/sandbox/eval/js/babel-parser.js index edd85172d0e..45315562442 100644 --- a/src/sandbox/eval/js/babel-parser.js +++ b/src/sandbox/eval/js/babel-parser.js @@ -62,7 +62,7 @@ export default function getBabelConfig( modules: Array, directories: Array, externals: Object, - depth: number, + depth: ?number, ) { const babelConfigModule = modules.find( m => m.title === '.babelrc' && !m.directoryShortid, diff --git a/src/sandbox/eval/js/index.js b/src/sandbox/eval/js/index.js index 330849b44e8..95f19563bda 100644 --- a/src/sandbox/eval/js/index.js +++ b/src/sandbox/eval/js/index.js @@ -14,12 +14,14 @@ const moduleCache = new Map(); * Deletes the cache of all modules that use module and module itself */ export function deleteCache(module: Module) { + // Delete own cache first, because with cyclic dependencies we could get a + // endless loop + moduleCache.delete(module.id); moduleCache.forEach(value => { if (value.requires.includes(module.id)) { deleteCache(value.module); } }); - moduleCache.delete(module.id); } const compileCode = ( @@ -45,12 +47,24 @@ function evaluate(code, require) { return Object.keys(exports).length > 0 ? exports : module.exports; } +/** + * Transpile & execute a JS file + * @param {*} mainModule The module to execute + * @param {*} modules All modules in the sandbox + * @param {*} directories All directories in the sandbox + * @param {*} externals A list of dependency with a mapping to dependencPath -> module id + * @param {*} depth The amount of requires we're deep in + * @param {*} parentModules If this is a module that's required, the parents that execute it + * are here (so if a requires b and b is executed, this will be [a]). + * This is required for cyclic dependency checks + */ export default function evaluateJS( mainModule: Module, modules: Array, directories: Array, externals: { [path: string]: string }, - depth: number, + depth: ?number, + parentModules: Array, ) { try { const requires = []; @@ -77,9 +91,18 @@ export default function evaluateJS( // Check if this module has been evaluated before, if so return that const cache = moduleCache.get(module.id); + // This is a cyclic dependency, we should return an empty object for first + // execution according to node spec + if (parentModules.includes(module) && !cache) { + return {}; + } + return cache ? cache.exports - : evalModule(module, modules, directories, externals, depth + 1); + : evalModule(module, modules, directories, externals, depth + 1, [ + ...parentModules, + mainModule, + ]); }; const babelConfig = getBabelConfig( From bc233e47c7171bd9ee624e46e634b12e3647a454 Mon Sep 17 00:00:00 2001 From: Ives van Hoorne Date: Mon, 10 Jul 2017 00:08:10 +0200 Subject: [PATCH 2/2] Tests --- src/sandbox/eval/index.test.js | 66 +++++++++++++++++++++++++++++++++- src/sandbox/eval/js/index.js | 4 +++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/sandbox/eval/index.test.js b/src/sandbox/eval/index.test.js index 031ca557ea5..571b8281679 100644 --- a/src/sandbox/eval/index.test.js +++ b/src/sandbox/eval/index.test.js @@ -1,8 +1,13 @@ import evaller from './'; +import { clearCache } from './js'; describe('eval', () => { // just evaluate if the right evallers are called describe('js', () => { + beforeEach(() => { + clearCache(); + }); + test('default es exports', () => { const mainModule = { title: 'test.js', @@ -60,7 +65,66 @@ describe('eval', () => { }; expect(evaller(mainModule, [mainModule, secondModule], [])).toEqual({ - default: 3, + default: { default: 3 }, + }); + }); + + describe('cyclic dependencies', () => { + it('returns an object as cyclic dependency', () => { + const moduleA = { + title: 'a.js', + shortid: '1', + code: ` + import b from './b'; + export default b; + `, + }; + + const moduleB = { + title: 'b.js', + shortid: '2', + code: ` + import a from './a'; + export default a; + `, + }; + + expect(evaller(moduleA, [moduleA, moduleB], [])).toEqual({ + default: {}, + }); + }); + + it('returns an object in deep cyclic dependency', () => { + const moduleA = { + title: 'a.js', + shortid: '1', + code: ` + import b from './b'; + export default b; + `, + }; + + const moduleB = { + title: 'b.js', + shortid: '2', + code: ` + import c from './c'; + export default c; + `, + }; + + const moduleC = { + title: 'c.js', + shortid: '3', + code: ` + import a from './a'; + export default a; + `, + }; + + expect(evaller(moduleA, [moduleA, moduleB, moduleC], [])).toEqual({ + default: {}, + }); }); }); diff --git a/src/sandbox/eval/js/index.js b/src/sandbox/eval/js/index.js index 95f19563bda..0459dbb7933 100644 --- a/src/sandbox/eval/js/index.js +++ b/src/sandbox/eval/js/index.js @@ -10,6 +10,10 @@ import getBabelConfig from './babel-parser'; const moduleCache = new Map(); +export function clearCache() { + moduleCache.clear(); +} + /** * Deletes the cache of all modules that use module and module itself */