szwacz / fs-jetpack

Better file system API for Node.js
MIT License
777 stars 41 forks source link

Support tree-shaking via esm builds #89

Open jasonkuhrt opened 4 years ago

jasonkuhrt commented 4 years ago

We're currently working on tree-shaking Nexus builds. We're trying rollup and webpack. With rollup we've been hitting some issues with fs-jetpack, such as:

Error: 'inspect' is not exported by node_modules/fs-jetpack/main.js, imported by dist/main.js
    at error (/Users/jasonkuhrt/test-rollup-ts/node_modules/rollup/dist/shared/rollup.js:161:30)
    at Module.error (/Users/jasonkuhrt/test-rollup-ts/node_modules/rollup/dist/shared/rollup.js:15027:16)
    at handleMissingExport (/Users/jasonkuhrt/test-rollup-ts/node_modules/rollup/dist/shared/rollup.js:14926:28)
    at Module.traceVariable (/Users/jasonkuhrt/test-rollup-ts/node_modules/rollup/dist/shared/rollup.js:15422:24)
    at ModuleScope.findVariable (/Users/jasonkuhrt/test-rollup-ts/node_modules/rollup/dist/shared/rollup.js:13978:39)
    at Identifier$1.bind (/Users/jasonkuhrt/test-rollup-ts/node_modules/rollup/dist/shared/rollup.js:9881:40)
    at CallExpression$1.bind (/Users/jasonkuhrt/test-rollup-ts/node_modules/rollup/dist/shared/rollup.js:9514:23)
    at CallExpression$1.bind (/Users/jasonkuhrt/test-rollup-ts/node_modules/rollup/dist/shared/rollup.js:11956:15)
    at CallExpression$1.bind (/Users/jasonkuhrt/test-rollup-ts/node_modules/rollup/dist/shared/rollup.js:9510:31)
    at CallExpression$1.bind (/Users/jasonkuhrt/test-rollup-ts/node_modules/rollup/dist/shared/rollup.js:11956:15) {

We're still investigating but it would be great if fs-jetpack had ESM builds.

Would you be open to a PR for this?

It should just be a matter of something like this https://github.com/graphql-nexus/nexus/commit/0955d73557aaf12c507e9734684b8c739f0582a9, where there are two tsc build steps.

szwacz commented 4 years ago

Many questions: So am I understanding you correctly, that rollup does aggressive treeshaking on fs-jetpack and the library doesn't work after that? I assume it happens only when you use es6 modules? Is the problem only with inspect or with all the methods?

Generally speaking sure I'd welcome a PR to fix it :) But I'd also like to know more about the issue.

jasonkuhrt commented 4 years ago

Ah, I mistakenly thought this project was written in TypeScript, only now realizing it is just the spec modules.

Rollup has trouble with this module

"use strict";

const jetpack = require("./lib/jetpack");

module.exports = jetpack();

Is the problem only with inspect or with all the methods?

All

I assume it happens only when you use es6 modules?

Yeah, we are working with TypeScript with ES modules enabled.

and the library doesn't work after that?

Rollup is unable to tree shake fs-jetpack in the first place.

But I'd also like to know more about the issue.

Will share more info as we get it ourselves 👍