Use browser-resolve to ignore fs inlining

[fathyb]

browser-resolve has an option to skip fs and other modules. This PR makes the fs visitor check if fs is ignored before processing it.

This makes aws-sdk-js usable amongst other libraries using this feature.

Fixes #496 and maybe #474

[olivoil]

I tested the PR in a project importing aws-amplify which depends on aws-sdk-js. Seems to fix the issue for me. Thanks @fathyb !

[devongovett]

Cool! another option would be to just not error on non-static file paths (e.g. with variables), and just skip inlining. The only thing I'd be concerned about would be that it might reduce debuggability for legit coding errors. thoughts?

[fathyb]

I agree on the debuggability point, it could be frustrating to not understand why it does not inline. What about passing an additional option to fs.readFileSync, something like :

fs.readFileSync(Math.random().toString(), {
  parcel: false
})

That way it can be ignored on node.js and can hint the visitor if the call wasn't meant to be parsed. It might not be applicable on certain use case tho.

[devongovett]

I don't think we should be changing the API. Also that won't work for existing node_modules that are already broken with Parcel.

[fathyb]

Alright, then I'd recommend to display a warning when it happens. #448 could be useful here to silent warnings on some files.

[SheetJSDev]

Encountered the Could not statically evaluate fs call error while trying to import the xlsx module. It uses the browser field of package.json to suppress for browserify.

another option would be to just not error on non-static file paths (e.g. with variables), and just skip inlining.

@devongovett even if a specific fs call could be statically evaluated, this patch is still important. false entries in package.json browser field unambiguously communicate a desire to suppress behavior in a browser-based deployment. The general expectation from the perspective of library authors is that require("fs") would either error or evaluate to {} if suppressed.

Considering prior art, neither browserify nor webpack attach special meaning to the fs module; the filesystem meaning is conferred by modules like brfs. You can manually verify this with a basic fs.readFileSync example in browserify and webpack.

@fathyb I personally like the concise expression

let ignore = (((asset||{}).package||{}).browser||{}).fs === false

but the explicit form is probably clearer.

[sterproton]

i have problem that after use browser-resolve and it skip fs module
but i still want to use the fs module and it just become {}?? and after parcel build, the prod code throw error because fs module become {}

i use parcel to bundler my node.js server code