rich-harris / agadoo Goto Github PK

This is a very interesting concept!

It seems to report some modules as "Failed to treeshake" because, they will include some comments:

/* global HTMLElement */

// @flow

// @flow

Failed to tree-shake index.js

This example in particular is pretty common. If I use this file, the comment will be printed out:

/*
 * I am a comment that will be included.
 */

export function xyz () {
  // ...
}

Hello @Rich-Harris , I love the idea of running Agadoo on our CI server to make sure the library we are building is fully tree-shakable. However, while I believe it is currently tree-shakable (we tried it with webpack 4 and made sure bundles were correctly limited to what was needed and no more), Agadoo is throwing errors I do not really understand.

Here is a super simple simplification of our lib (after building):

dist
  ├── index.js
  ├──aaa/
  │   ├── index.js
  │   └── aaa.js
  └──bbb/
      ├── index.js
      └── bbb.js

There is a bunch of components, so imagine ccc, ddd folders in the same vein.

And here are the content of each files:

/* main index.js */
import aaa from "./aaa";
import bbb from "./bbb";
export { aaa, bbb };

/* index.js under folder aaa */
import aaa from "./aaa";
export { aaa };

/* aaa.js */
function aaa(a) {
  console.log("==> hello aaa: ", a);
}
export default aaa;

/* package.json */
(...)
"module": "dist/index.js",
(...)

With this setup, when I run node node_modules/.bin/agadoo the following error message is thrown:

(node:57176) UnhandledPromiseRejectionWarning: Error: Could not resolve './aaa' from dist/index.js
    at error (/Users/aversin/tmp/testing-agadoo/node_modules/rollup/dist/rollup.js:3365:15)
    at /Users/aversin/tmp/testing-agadoo/node_modules/rollup/dist/rollup.js:21504:25
    at process._tickCallback (internal/process/next_tick.js:68:7)
    at Function.Module.runMain (internal/modules/cjs/loader.js:745:11)
    at startup (internal/bootstrap/node.js:266:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:596:3)
(node:57176) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:57176) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Any idea what's wrong? Is it us and our weird sub-folder organization? Or is it Agadoo?
Any pointers would be great, thanks!!

PS: I tried with node 8.11.4 and 10.9.0

Cool project idea. 😄

I tried plugging it into one of my React libs this morning and noticed a failure that I didn't expect. I reduced to the following case in case you're interested:

✓ This passes:

var Foo;
Foo = function (props) {
};
export { Foo };

✓ This passes:

var Foo = function (props) {
};
Foo.defaultProps = {
};
export { Foo };

✘ This fails:

var Foo;
Foo = function (props) {
};
Foo.defaultProps = {
};
export { Foo };

Hi. This package idea is excellent!

Sadly, I have a small issue: I can't use this lib inside a Yarn workspace because it get the wrong package.json.

Say I have the following tree:

• package.json
• packages/
  • mylib/
    • package.json

If I run agadoo on for mylib like:

cd packages/mylib
yarn add -D agadoo
./node_module/.bin/agadoo

If get the following error message:

'PWD/packages/mylib' is imported by virtual:__agadoo__, but could not be resolved – treating it as an external dependency
import './';

Failed to tree-shake 

I'm probably missing something because it is rare for an executable to not have a help flag (<executable> --help or <executable> -h for help around the arguments, parameters and their descriptions).

When asking agadoo for help with agadoo -h, I get

Success! -h is fully tree-shakeable

which is probably unintended.

This is more of a question: I am wondering how external libraries can be handled? When the module I am testing imports some other external library like so:

import test from 'test';

The Rollup's code output contains that import statement (i.e., code: 'import \'test\';\n'). Hence, agadoo doesn't attest full shakability.

I guess my question is whether this is expected and if agadoo would just have to take this into account when testing for shakability or if something else needs to be adjusted.

I am developping library of components with rollup and I am using agadoo tool to check if it is tree shakable.
Maybe this question is off the topic, but when I use the below code the library is not tree shakable. And I don't understand why.

import React from 'react';

class Welcome extends React.PureComponent {
    render() {
        return <h1>Hello, {this.props.name}</h1>;
    }
}

export default Welcome

On the other hand, when I don't use the React Component, it stays tree shakable, so what's wrong with the React component?

import React from 'react';

function Welcome(props) {
  return <h1>Hello, {props.name}</h1>;
}

export default Welcome

We have a package which needs some custom Rollup settings. Instead of forking agadoo, I'd like a way to customize the config.

Passing in a config file from the cli -- either a base config for agadoo to add to, or a function that receives agadoo's config and returns the final config -- would work well for me. Does this sound reasonable?

I'm happy to submit a PR, if you'd like.