Rich-Harris / agadoo

Check whether a package is tree-shakeable
MIT License
535 stars 17 forks source link

UnhandledPromiseRejectionWarning: Error: Could not resolve './aaa' from dist/index.js #2

Closed aversini closed 1 year ago

aversini commented 6 years ago

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

adrienpoly commented 3 years ago

I had a similar problem and could solve it by slightly modifying my imports

/* main index.js */
import aaa from "./aaa/index"; // add index
import bbb from "./bbb/index";  // add index
export { aaa, bbb };
mfrancekovic commented 3 years ago

I have the same issue. The solution which @adrienpoly suggests did fix it. Having said that it would be wonderful not to have to add /index in each import.

icy0307 commented 1 year ago

I am facing the same issue. What I am trying todo is check every packages my project using are tree-shakeable, and forbid to add new package if it is not shakable. @Rich-Harris Is this a rollup problem?

Rich-Harris commented 1 year ago

Is this a rollup problem?

No. Agadoo assumes it's testing an ESM entry point. ESM requires you to be specific about what file you're importing; consequently, so does Rollup. Rollup is in fact too liberal — for the file to be valid ESM it must specify the .js file extension — but that's a design mistake that would be hard to undo at this point.

If it's failing because it's not finding an index file imported from the entry point, it means that the import is invalid in the first place, and won't run natively in Node. For some projects that's fine (i.e. if the code will only ever be consumed via tooling) but since best practice is to be specific about what files you import, Rollup/Agadoo's behaviour won't change.