Support for cyclic dependencies (codesandbox#42)

* Support for cyclic dependencies

* Tests

Author: CompuIves

src/sandbox/eval/index.js

@@ -14,29 +14,65 @@ function doEval(
   directories: Array<Directory>,
   externals: Object,
   depth: ?number,
+  parentModule: ?Module,
 ) {
   const html = /\.html$/;
   const css = /\.css$/;
   const json = /\.json$/;
   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,14 +85,22 @@ const evalModule = (
   directories: Array<Directory>,
   externals: Object,
   depth: number = 0,
+  parentModule: Array<Module> = [],
 ) => {
   if (depth > MAX_DEPTH) {
     throw new Error(
       `Exceeded the maximum require depth of ${MAX_DEPTH}, there are probably two files depending on eachother.`,
     );
   }
   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;

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: {},
+        });
       });
     });
 

src/sandbox/eval/js/babel-parser.js

@@ -62,7 +62,7 @@ export default function getBabelConfig(
   modules: Array<Module>,
   directories: Array<Directory>,
   externals: Object,
-  depth: number,
+  depth: ?number,
 ) {
   const babelConfigModule = modules.find(
     m => m.title === '.babelrc' && !m.directoryShortid,

src/sandbox/eval/js/index.js

@@ -10,16 +10,22 @@ 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
  */
 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 +51,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<Module>,
   directories: Array<Directory>,
   externals: { [path: string]: string },
-  depth: number,
+  depth: ?number,
+  parentModules: Array<Module>,
 ) {
   try {
     const requires = [];
@@ -77,9 +95,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(