FormidableLabs / inspectpack

An inspection tool for Webpack frontend JavaScript bundles.
MIT License
592 stars 20 forks source link

Add `--action=parse` report and misc fixes. #9

Closed ryan-roemer closed 8 years ago

ryan-roemer commented 8 years ago

There's a conspicuous note of needing a babel function to return true for matching multiple exports (which we define as a module.exports field that is set from a require() of another file -- it's not just multiple fields on module.exports).

I've got a sample simple bundle up at: https://gist.github.com/ryan-roemer/c2b507ef0e17c392b9f09b7a03e1371c that you can use to testing things out.

$ bin/inspectpack.js \
  --action=parse \
  --bundle="/PATH/TO/bundle.multiple-exports.js" \
  --format=text \
  --suspect-parses

Feel free to commit directly to the SUSPECT_PARSES.MULTIPLE_EXPORT function, or branch my branch -- whichever is easier.

Thanks!

/cc @divmain @tptee

ryan-roemer commented 8 years ago

UPDATE: I've added scenarios e and f as matches to https://gist.github.com/ryan-roemer/c2b507ef0e17c392b9f09b7a03e1371c Let the fun continue!

ryan-roemer commented 8 years ago

@divmain @tptee - any updates on this one? I'd love to use this on a client project soon, so would be great if I had a ballpark of how long this is going to take. Thanks!

divmain commented 8 years ago

I've added re-export detection for simple cases. However, there are a number of cases (including the default Babel transformation for ES6 imports/exports) that are not covered here and which, I believe, will be very difficult to support.

The introspection necessary to traverse the AST, and determine if and where a particular variable depends on a particular import is prohibitively difficult. If you restricted users to ES6 module syntax and inspected the codebase prior to webpack build, this would be an easier problem to solve. As is, I think it likely that any attempt at supporting anything beyond the base case will be a brittle solution at best.

Some problematic examples:

// CASE 1: `export { bar } from "foo";`

var _foo = require("foo");

Object.defineProperty(exports, "bar", {
  enumerable: true,
  get: function () {
    return _foo.bar;
  }
});

// CASE 2: assign

Object.assign(exports, {
  foo: __webpack_require__("foo")
});

// CASE 3: `export * from "foo"`

Object.defineProperty(exports, "__esModule", {
  value: true
});

var _foo = __webpack_require__("foo");

Object.keys(_foo).forEach(function (key) {
  if (key === "default") return;
  Object.defineProperty(exports, key, {
    enumerable: true,
    get: function () {
      return _foo[key];
    }
  });
});

// CASE 4: IIFE

exports.foo = (function () {
  return __webpack_require__("foo");
})();

// CASE 5: require property

exports.foo = __webpack_require__("foo").foo;
divmain commented 8 years ago

Example output:

## Summary

* Bundle:
    * Path:                example.js
    * Num Matches:         3
    * Num Unique Files:    3
    * Num All Files:       3
    * Custom Parses:
    * Suspect Parses:
        * MULTIPLE_EXPORT

## Matches

* ./lib/mod-a.js
    * Num Matches:         1
    * Num Files Matched:   1

    * 1: ./lib/mod-a.js
        * Matches: 1
            * MULTIPLE_EXPORT:
              module.exports = {
                first: __webpack_require__(/*! ./first */ 2),
                second: __webpack_require__(/*! ./second */ 3)
              };

* ./lib/mod-b.js
    * Num Matches:         1
    * Num Files Matched:   1

    * 4: ./lib/mod-b.js
        * Matches: 1
            * MULTIPLE_EXPORT:
              module.exports.first = __webpack_require__(/*! ./first */ 2);
              // ...
              module.exports.second = __webpack_require__(/*! ./second */ 3);

* ./lib/mod-c.js
    * Num Matches:         1
    * Num Files Matched:   1

    * 5: ./lib/mod-c.js
        * Matches: 1
            * MULTIPLE_EXPORT:
              module.exports = {
                first: __webpack_require__(/*! ./first */ 2)
              };
              // ...
              module.exports.second = __webpack_require__(/*! ./second */ 3);
divmain commented 8 years ago

@ryan-roemer, this is ready for your review.

ryan-roemer commented 8 years ago

@divmain -- Lovely! Kicking the tires on this on today.

divmain commented 8 years ago

One comment, the rest LGTM. (it is the comment right above this, GitHub thinks it doesn't need to display it)

ryan-roemer commented 8 years ago

Released in inspectpack@0.4.0. Thanks @divmain !