feat: proper source map rewrite. refactor trace pipeline

closes #6

Author: 3cp

package.json

@@ -18,8 +18,8 @@
     "postversion": "git push && git push --tags && npm publish",
     "pretest": "npm run lint",
     "prepare": "del-cli dist && babel src -d dist",
-    "nodejs-test": "tape -r @babel/register \"test/**/*.spec.js\" | tap-dot",
-    "coverage": "nyc --reporter=lcov tape -r @babel/register \"test/**/*.spec.js\" | tap-dot",
+    "nodejs-test": "tape -r @babel/register test/hide-log.js \"test/**/*.spec.js\" | tap-dot",
+    "coverage": "nyc --reporter=lcov tape -r @babel/register test/hide-log.js \"test/**/*.spec.js\" | tap-dot",
     "browser-test": "browserify -t babelify test/all-browser-spec.js | tape-run | tap-dot",
     "test": "npm run nodejs-test && npm run browser-test"
   },
@@ -40,7 +40,7 @@
   },
   "homepage": "https://github.com/dumberjs/dumber#readme",
   "dependencies": {
-    "@babel/core": "^7.2.2",
+    "@babel/core": "^7.3.4",
     "@babel/plugin-syntax-dynamic-import": "^7.2.0",
     "@babel/plugin-transform-modules-commonjs": "^7.2.0",
     "ast-matcher": "^1.0.2",
@@ -51,6 +51,7 @@
     "dumber-module-loader": "^0.16.0",
     "escope": "^3.6.0",
     "mkdirp": "^0.5.1",
+    "modify-code": "^0.2.1",
     "node-fetch": "^2.3.0",
     "node-libs-browser": "^2.2.0",
     "readable-stream": "^2.3.6",
@@ -59,17 +60,18 @@
   },
   "devDependencies": {
     "@babel/cli": "^7.2.3",
-    "@babel/preset-env": "^7.3.1",
+    "@babel/preset-env": "^7.3.4",
     "@babel/register": "^7.0.0",
     "babel-eslint": "^10.0.1",
     "babelify": "^10.0.0",
     "browserify": "^16.2.3",
     "del-cli": "^1.1.0",
     "eslint": "^5.15.1",
-    "nyc": "^13.2.0",
-    "standard-changelog": "^2.0.6",
+    "nyc": "^13.3.0",
+    "sourcemap-codec": "^1.4.4",
+    "standard-changelog": "^2.0.7",
     "tap-dot": "^2.0.0",
-    "tape": "^4.9.2",
+    "tape": "^4.10.1",
     "tape-run": "^5.0.0"
   }
 }

src/index.js

@@ -1,6 +1,5 @@
 import trace from './trace';
 import {cleanPath, parse, mapId} from 'dumber-module-loader/dist/id-utils';
-import alias from './transformers/alias';
 import defaultPackageFileReader from './package-file-reader/default';
 import PackageReader from './package-reader';
 import Package from './package';
@@ -9,7 +8,6 @@ import {info, error, warn} from './log';
 import {generateHash, stripJsExtension, resolvePackagePath, contentOrFile} from './shared';
 import * as cache from './cache/default';
 import path from 'path';
-import mergeTransformed from './transformers/merge';
 import ModulesDone from './modules-done';
 import ModulesTodo from './modules-todo';
 
@@ -167,7 +165,9 @@ export default class Bundler {
   }
 
   _capture(tracedUnit) {
-    this._unitsMap[tracedUnit.path] = tracedUnit;
+    let key = tracedUnit.path;
+    if (tracedUnit.packageName) key = tracedUnit.packageName + ':' + key;
+    this._unitsMap[key] = tracedUnit;
 
     // mark as done.
     this._modules_done.addUnit(tracedUnit);
@@ -194,10 +194,7 @@ export default class Bundler {
       p = p.then(() => this.packageReaderFor(pkg)).then(reader => {
         if (!pkg.lazyMain) {
           return reader.readMain()
-          .then(unit => this.capture(unit))
-          .then(tracedUnit => {
-            this._ensureNpmAlias(tracedUnit, pkg.name);
-          });
+          .then(unit => this.capture(unit));
         }
       });
     });
@@ -222,31 +219,6 @@ export default class Bundler {
     })
   }
 
-  // this ensures alias to package main, and alias to direct require
-  // to some browser replacement.
-  // e.g. readable-stream/readable -> readable-stream/readable-browser
-  _ensureNpmAlias(tracedUnit, id) {
-    const defined = tracedUnit.defined;
-    let toId;
-    if (Array.isArray(defined)) {
-      const ds = defined.map(d => parse(d).bareId);
-      if (ds.indexOf(id) !== -1) return;
-      toId = ds[0];
-    } else if (typeof defined === 'string') {
-      toId = parse(defined).bareId;
-    }
-
-    // alias to main is also created here
-    if (!toId) warn('no defined module found in ' + tracedUnit.path);
-
-    // only create alias when defined id is not same as package name
-    if (toId !== id && toId !== tracedUnit.packageName) {
-      const aliasResult = alias(id, toId);
-      mergeTransformed(tracedUnit, aliasResult);
-      this._modules_done.addUnit(tracedUnit);
-    }
-  }
-
   _supportInjectCssIfNeeded() {
     if (!this._modules_todo.needCssInjection || !this._injectCss || this._isInjectCssTurnedOn) {
       return Promise.resolve();
@@ -280,11 +252,16 @@ export default class Bundler {
     const requiredBy = opts.requiredBy;
     const parsedId = parse(mapId(id, this._paths));
 
+    if (this._modules_done.has(parsedId.bareId, checkUserSpace, checkPackageSpace)) {
+      return Promise.resolve();
+    }
+
     // TODO add a callback point to fillup missing local dep.
     // This is needed by dumberify.
     if (checkUserSpace && !checkPackageSpace) {
       // detected missing local dep
       warn(`local dependency ${parsedId.bareId} (requiredBy ${requiredBy.join(', ')}) is missing`);
+      return Promise.resolve();
     }
 
     return new Promise(resolve => {
@@ -344,9 +321,6 @@ export default class Bundler {
       return this.packageReaderFor(stub || {name: packageName})
         .then(reader => resource ? reader.readResource(resource) : reader.readMain())
         .then(unit => this.capture(unit))
-        .then(tracedUnit => {
-          this._ensureNpmAlias(tracedUnit, bareId);
-        })
         .catch(err => {
           error('Resolving failed for module ' + bareId);
           error(err);
@@ -357,8 +331,8 @@ export default class Bundler {
   _unitsForBundle(bundle) {
     let units = [];
 
-    Object.keys(this._unitsMap).forEach(filePath => {
-      const unit = this._unitsMap[filePath];
+    Object.keys(this._unitsMap).forEach(key => {
+      const unit = this._unitsMap[key];
       if (unit.bundle === bundle) units.push(unit);
     });
 

src/modules-todo.js

@@ -6,9 +6,8 @@ const PACKAGE = '1';
 const USER_OR_PACKAGE = '2';
 
 export default class ModulesTodo {
-  constructor(modulesDone) {
+  constructor() {
     this._reset();
-    this._modulesDone = modulesDone;
   }
 
   _reset() {
@@ -68,27 +67,14 @@ export default class ModulesTodo {
       return Promise.resolve();
     }
 
-    const actualTodos = Object.create(null);
-
-    Object.keys(this.todos).sort().forEach(key => {
-      const [space, id] = spaceAndId(key);
-
-      let checkUserSpace = true;
-      let checkPackageSpace = true;
-      if (space === USER) checkPackageSpace = false;
-      else if (space === PACKAGE) checkUserSpace = false;
-
-      if (!this._modulesDone.has(id, checkUserSpace, checkPackageSpace)) {
-        actualTodos[key] = this.todos[key];
-      }
-    });
+    const {todos} = this;
     this._reset();
 
     let p = Promise.resolve();
 
-    Object.keys(actualTodos).forEach(key => {
+    Object.keys(todos).sort().forEach(key => {
       const [space, id] = spaceAndId(key);
-      const requiredBy = actualTodos[key];
+      const requiredBy = todos[key];
 
       p = p.then(() => cb(id, {
         user: space !== PACKAGE,

src/package-file-reader/default.js

@@ -56,6 +56,15 @@ export default function (packageConfig, mock) {
         }
         throw err;
       }
-    );
+    ).then(unit => {
+      if (filePath === 'package.json' || filePath === './package.json') {
+        const meta = JSON.parse(unit.contents);
+        if (meta.name !== name) {
+          meta.name = name;
+          unit.contents = JSON.stringify(meta);
+        }
+      }
+      return unit;
+    });
   });
 }

src/package-file-reader/jsDelivr.js

@@ -25,9 +25,7 @@ function fetchContent(fetchApi, url) {
 }
 
 function fetchJson(fetchApi, url) {
-  return fetchContent(fetchApi, url).then(function(text) {
-    return JSON.parse(text);
-  });
+  return fetchContent(fetchApi, url).then(text => JSON.parse(text));
 }
 
 // use jsDelivr to find npm package files
@@ -70,11 +68,15 @@ export default function (packageConfig, mock) {
 
     return function(filePath) {
       const fp = packagePath + '/' + filePath;
-      return fetchContent(_fetch, fp).then(function (text) {
-        return {
-          path: fp,
-          contents: text
+      return fetchContent(_fetch, fp).then(text => {
+        if (filePath === 'package.json' || filePath === './package.json') {
+          const meta = JSON.parse(text);
+          if (meta.name !== name) {
+            meta.name = name;
+            text = JSON.stringify(meta);
+          }
         }
+        return {path: fp, contents: text};
       });
     }
   });

src/package-reader.js

@@ -1,6 +1,5 @@
 import {ext, parse, resolveModuleId, relativeModuleId} from 'dumber-module-loader/dist/id-utils';
 import {stripJsExtension, isPackageName, stripSourceMappingUrl, getSourceMap} from './shared';
-import replace from './transformers/replace';
 import {error} from './log';
 import path from 'path';
 
@@ -40,7 +39,16 @@ export default class PackageReader {
   }
 
   readMain() {
-    return this.ensureMainPath().then(() => this._readFile(this.mainPath));
+    return this.ensureMainPath()
+      .then(() => this._readFile(this.mainPath))
+      .then(unit => {
+        // add alias from package name to main file module id.
+        // but don't add alias from "foo" to "foo/main.css".
+        if (ext(unit.moduleId) === ext(this.name)) {
+          unit.alias = this.name;
+        }
+        return unit;
+      });
   }
 
   readResource(resource) {
@@ -75,6 +83,15 @@ export default class PackageReader {
       }
 
       return findResource();
+    }).then(unit => {
+      const requested = this.name + '/' + resource;
+      if (unit.moduleId !== requested) {
+        // The requested id could be different from real id.
+        // for example, some browser replacement.
+        // e.g. readable-stream/readable -> readable-stream/readable-browser
+        unit.alias = requested;
+      }
+      return unit;
     });
   }
 
@@ -83,9 +100,8 @@ export default class PackageReader {
     return this.loadFile(filePath).then(file => {
       const moduleId = this.name + '/' + parse(stripJsExtension(filePath)).bareId;
 
+      let replacement;
       if (ext(filePath) === '.js') {
-        const replacement = {};
-
         Object.keys(this.browserReplacement).forEach(key => {
           const target = this.browserReplacement[key];
           const baseId = this.name + '/index';
@@ -102,19 +118,22 @@ export default class PackageReader {
             targetModule = '__ignore__';
           }
 
+          if (!replacement) replacement = {};
           replacement[sourceModule] = targetModule;
         });
-
-        file.contents = replace(file.contents, replacement);
       }
 
-      return {
+      const unit = {
         path: file.path.replace(/\\/g, '/'),
         contents: stripSourceMappingUrl(file.contents),
         moduleId,
         packageName: this.name,
         sourceMap: getSourceMap(file.contents, file.path)
       };
+
+      // the replacement will be picked up by transformers/replace.js
+      if (replacement) unit.replacement = replacement;
+      return unit;
     });
   }