revert: feat: be smart on onRequire stub's module space

This reverts commit 3851939.

Author: 3cp

spec/bundler.spec.js

@@ -427,7 +427,7 @@ test('Bundler replaces deps when onRequire returns array', t => {
   .then(t.end);
 });
 
-test('Bundler supports user space implementation returned by onRequire', t => {
+test('Bundler supports implementation returned by onRequire', t => {
   const fakeFs = {
     'node_modules/dumber-module-loader/dist/index.js': 'dumber-module-loader',
     'node_modules/loo/package.json': '{"name":"loo","main":"loo"}',
@@ -469,51 +469,6 @@ test('Bundler supports user space implementation returned by onRequire', t => {
   .then(t.end);
 });
 
-test('Bundler supports package space implementation returned by onRequire', t => {
-  const fakeFs = {
-    'node_modules/dumber-module-loader/dist/index.js': 'dumber-module-loader',
-    'node_modules/loo/package.json': '{"name":"loo","main":"loo"}',
-    'node_modules/loo/loo.js': '',
-    'node_modules/bar/package.json': '{"name":"bar","main":"bar"}',
-    'node_modules/bar/bar.js': 'foo',
-  };
-  const bundler = createBundler(fakeFs, {
-    onRequire(moduleId) {
-      // onRequire can return a Promise to resolve to false, array, or string.
-      if (moduleId === 'foo') return Promise.resolve("loo"); // "loo" will be processed by mockTrace
-    }
-  });
-
-  Promise.resolve()
-  .then(() => bundler.capture({path: 'src/app.js', contents: 'bar', moduleId: 'app'}))
-  .then(() => bundler.resolve())
-  .then(() => bundler.bundle())
-  .then(
-    bundleMap => {
-      t.deepEqual(bundleMap, {
-        'entry-bundle': {
-          files: [
-            {contents: 'dumber-module-loader;'},
-            {contents: 'define.switchToUserSpace();'},
-            {path: 'src/app.js', contents: "define('app',[\"bar\"],1);", sourceMap: undefined},
-            {contents: 'define.switchToPackageSpace();'},
-            {path: 'node_modules/bar/bar.js', contents: "define('bar/bar',[\"foo\"],1);define('bar',['bar/bar'],function(m){return m;});", sourceMap: undefined},
-            {path: '__on_require__/foo.js', contents: "define('foo',[\"loo\"],1);", sourceMap: undefined},
-            {path: 'node_modules/loo/loo.js', contents: "define('loo/loo',[],1);define('loo',['loo/loo'],function(m){return m;});", sourceMap: undefined},
-            {contents: 'define.switchToUserSpace();'},
-          ],
-          config: {
-            baseUrl: '/dist',
-            bundles: {}
-          }
-        }
-      })
-    },
-    err => t.fail(err.stack)
-  )
-  .then(t.end);
-});
-
 test('Bundler swallows onRequire exception', t => {
   const fakeFs = {
     'node_modules/dumber-module-loader/dist/index.js': 'dumber-module-loader',

src/index.js

@@ -43,11 +43,10 @@ export default class Bundler {
     // turn on injection of css (inject onto html head)
     if (opts.injectCss || opts.injectCSS) this._injectCss = true;
 
-    this._unitsMap = Object.create(null);
-    this._user_modules_done = new Set();
-    this._package_modules_done = new Set();
-    this._moduleIds_todo = Object.create(null);
-    this._readersMap = Object.create(null);
+    this._unitsMap = {};
+    this._moduleId_done = new Set();
+    this._moduleIds_todo = new Set();
+    this._readersMap = {};
     this._locator = opts.packageLocator || defaultPackageLocator;
 
     // baseUrl default to "dist"
@@ -81,7 +80,7 @@ export default class Bundler {
   }
 
   packageReaderFor(packageConfig) {
-    if (this._readersMap[packageConfig.name]) {
+    if (this._readersMap.hasOwnProperty(packageConfig.name)) {
       return Promise.resolve(this._readersMap[packageConfig.name]);
     }
 
@@ -125,45 +124,26 @@ export default class Bundler {
     );
   }
 
-  _didUserModule(id) {
+  _addToDone(id) {
     if (typeof id === 'string') {
-      this._user_modules_done.add(id);
+      this._moduleId_done.add(id);
     } else if (Array.isArray(id)) {
-      id.forEach(d => this._user_modules_done.add(d));
-    }
-  }
-
-  _didPackageModule(id) {
-    if (typeof id === 'string') {
-      this._package_modules_done.add(id);
-    } else if (Array.isArray(id)) {
-      id.forEach(d => this._package_modules_done.add(d));
-    }
-  }
-
-  _addTodo(d, isPackageSpace) {
-    if (this._moduleIds_todo[d]) {
-      if (isPackageSpace) this._moduleIds_todo[d] = 2;
-    } else {
-      this._moduleIds_todo[d] = isPackageSpace ? 2 : 1
+      id.forEach(d => this._moduleId_done.add(d));
     }
   }
 
   _capture(tracedUnit) {
     this._unitsMap[tracedUnit.path] = tracedUnit;
 
     // mark as done.
-    if (tracedUnit.packageName) {
-      this._didPackageModule(tracedUnit.moduleId);
-      this._didPackageModule(tracedUnit.defined);
-    } else {
-      this._didUserModule(tracedUnit.moduleId);
-      this._didUserModule(tracedUnit.defined);
-    }
+    this._addToDone(tracedUnit.moduleId);
+    this._addToDone(tracedUnit.defined);
 
-    // mark todo. beware we didn't check whether the id is in _user/package_modules_done.
+    // mark todo. beware we didn't check whether the id is in _moduleId_done.
     // they will be checked during resolve phase.
-    tracedUnit.deps.forEach(d => this._addTodo(d, tracedUnit.packageName));
+
+    // console.log('_capture ' + tracedUnit.moduleId + ' deps ' + tracedUnit.deps);
+    tracedUnit.deps.forEach(d => this._moduleIds_todo.add(d));
 
     const bundle = this.bundleOf(tracedUnit);
     // mark related bundle dirty
@@ -218,7 +198,7 @@ export default class Bundler {
   // to some browser replacement.
   // e.g. readable-stream/readable -> readable-stream/readable-browser
   _ensureNpmAlias(tracedUnit, id) {
-    if (this._package_modules_done.has(id)) return;
+    if (this._moduleId_done.has(id)) return;
 
     const defined = tracedUnit.defined;
     let toId;
@@ -237,7 +217,7 @@ export default class Bundler {
     if (toId !== id && toId !== tracedUnit.packageName) {
       const aliasResult = alias(id, toId);
       mergeTransformed(tracedUnit, aliasResult);
-      this._didPackageModule(aliasResult.defined);
+      this._addToDone(aliasResult.defined);
     }
   }
 
@@ -253,31 +233,21 @@ export default class Bundler {
   }
 
   resolve() {
-    // todo is always to be resolved in package space.
     let todo = [];
-
     return this._supportInjectCssIfNeeded()
     .then(() => this._resolvePrependsAndAppends())
     .then(() => this._resolveExplicitDepsIfNeeded())
     .then(() => {
       const consults = [];
-      const rawTodo = JSON.parse(JSON.stringify(this._moduleIds_todo));
-      this._moduleIds_todo = Object.create(null);
+      const rawTodo = Array.from(this._moduleIds_todo);
+      this._moduleIds_todo.clear();
+
 
-      Object.keys(rawTodo).forEach(id => {
-        const isPackageSpace = rawTodo[id] === 2;
+      // console.log('_moduleIds_todo', this._moduleIds_todo);
+      rawTodo.forEach(id => {
         const parsedId = parse(id);
         const possibleIds = nodejsIds(parsedId.bareId);
-
-        const isAvailabe = possibleIds.some(id => {
-          if (isPackageSpace) {
-            return this._package_modules_done.has(id);
-          } else {
-            return this._user_modules_done.has(id) || this._package_modules_done.has(id);
-          }
-        });
-
-        if (isAvailabe) return;
+        if (possibleIds.some(id => this._moduleId_done.has(id))) return;
 
         const j = new Promise(resolve => {
           resolve(this._onRequire && this._onRequire(parsedId.bareId, parsedId));
@@ -297,8 +267,7 @@ export default class Bundler {
               return this.capture({
                 path: '__on_require__/' + parsedId.bareId + (parsedId.ext ? '' : '.js'),
                 contents: result,
-                moduleId: parsedId.bareId,
-                packageName: isPackageSpace ? parsedId.parts[0] : ''
+                moduleId: parsedId.bareId
               });
             }
 
@@ -356,7 +325,7 @@ export default class Bundler {
       return p;
     })
     .then(() => {
-      if (Object.keys(this._moduleIds_todo).length) {
+      if (this._moduleIds_todo.size) {
         return this.resolve();
       }
     });