ethersphere / bee-js

Javascript client library for connecting to Bee decentralised storage
https://bee-js.ethswarm.org/
BSD 3-Clause "New" or "Revised" License
60 stars 27 forks source link

NodeJS modules are needed to be polyfilled in browser builds #575

Open mattiaz9 opened 2 years ago

mattiaz9 commented 2 years ago

When I try to start my react application it gives me this error:

✘ [ERROR] No matching export in "browser-external:stream" for import "Readable"

    node_modules/@ethersphere/bee-js/dist/mjs/utils/stream.js:1:9:
      1 │ import { Readable as NodeReadableNative } from 'stream';
        ╵          ~~~~~~~~

I'm using Vitejs with typescript BTW, so I'm not shimming node dependencies.

AuHau commented 2 years ago

Thanks for the report. Just to be sure have you tried it with the latest patch 3.3.1?

mattiaz9 commented 2 years ago

Thanks for the report. Just to be sure have you tried it with the latest patch 3.3.1?

Yes

AuHau commented 2 years ago

@mattiaz9 I was exploring your issue and it seems that unfortunately, this is a problem of ViteJS because it does not handle correctly packageJson.browser field: https://github.com/vitejs/vite/issues/7576

I am afraid there is nothing we can do on our side until this is resolved.

mattiaz9 commented 2 years ago

I am afraid there is nothing we can do on our side until this is resolved.

Do you actually need nodejs modules in the browser?

Shimming node packages is a bad practice anyway, and many bundlers (if not all), stopped injecting them automatically.

If the 3.2 version works I assume this practice wasn't used before and probably isn't needed, at least for the browser version of this package.

AuHau commented 2 years ago

Bee-js aims to be a platform agnostic library, which internally uses browser-first APIs so most of the polyfilling is actually happening on Node's side, but still, there needs to be differentiation on how the data are handled/processed in browser and node as different APIs are available across these environments. For this to happen there is nowadays a pretty standardized approach using packageJson.browser field where you can specify which files should be used where and then bundlers appropriately use files for given targets. The fact that Vite does not handle this correctly is unfortunate.

The reason why this issue has surfaced now is that we have changed how we bundle bee-js, where historically we have bundled all the files for all environments (including Node) using webpack, which then handled all this for you, but also bundles all the dependencies and everything else into one big file, which does not really make sense as it prevents for bee-js users to utilize their bundlers to do proper optimizations around dependencies de-duplication, tree shaking etc and generally does not make sense for Node environment, so we have dropped that and we now uses simple tsc for just basic source code processing from TypeScript for the library's main entrypoint.

One thing you could try to do is to use instead of our tsc build that relies upon correct behavior of user's bundler, to use the bundled version from webpack for browsers that we also distribute. Not sure exactly how it would work, but you could try to import @ethersphere/bee-js/index.browser.js, if you use TypeScript then you would have to workaround typings somehow, but that should be possible as well.

AuHau commented 2 years ago

@mattiaz9 the issue in Vite was fixed so in the next release please test it ;-) https://github.com/vitejs/vite/pull/10314

mattiaz9 commented 2 years ago

@AuHau I tried with the last 2 versions 3.1.6 and 3.1.7, as well as the beta 3.2.0-beta.0, but I get this error:

'Readable' is not exported by __vite-browser-external, imported by node_modules/@ethersphere/bee-js/dist/mjs/utils/stream.js
file: /Users/mattia/Developer/Webapps/test-beejs/node_modules/@ethersphere/bee-js/dist/mjs/utils/stream.js:1:9
1: import { Readable as NodeReadableNative } from 'stream';
            ^
2: import { isStrictlyObject } from "./type.js";
3: import { ReadableStream } from 'web-streams-polyfill';
AuHau commented 2 years ago

Hmm, well it seems like others reports still the problem existing: https://github.com/vitejs/vite/issues/7576#issuecomment-1272276336

Lets see how it evolves...