FuelLabs / fuels-ts

Fuel Network Typescript SDK
https://docs.fuel.network/docs/fuels-ts/
Apache License 2.0
44.4k stars 1.32k forks source link

Add `Buffer` compatibility for browsers #765

Open arboleya opened 1 year ago

arboleya commented 1 year ago

We need to support Buffer usage inside the Browsers.

For example, someone using React with Vite might fall into this error:

error

And then having to resort to custom mapping configurations such by installing the buffer package:

pnpm install buffer

And then configuring custom mappings in vite.config.js:

import { defineConfig } from 'vite'
import react from '@vitejs/plugin-react'

// https://vitejs.dev/config/
export default defineConfig({
  plugins: [react()],
  resolve: { alias: { buffer: 'node_modules/buffer/index.d.ts' } },
})
arboleya commented 1 year ago

This might relate to the following PR, in the sense that the Buffer usages should fail for the browser group:

cc @Dhaiwat10

Dhaiwat10 commented 1 year ago

@arboleya none of our browser tests are failing in #728. I am using puppeteer to run the tests in a browser. Will need to investigate the difference between that environment and Vite. Do you have a repo containing a reproducible example that I can run locally to see this Vite error?

arboleya commented 1 year ago

@Dhaiwat10 I created this issue from a conversation with @Braqzen.

@Braqzen Do you happen to have a simple repro for this problem?

Braqzen commented 1 year ago

@Dhaiwat10 I created this issue from a conversation with @Braqzen.

@Braqzen Do you happen to have a simple repro for this problem?

The steps are something like this (hopefully a repro):

  1. pnpm create vite@latest ui --template react-ts
  2. mkdir /ui/src/contract-typegen
  3. cd /ui
  4. pnpm install
  5. pnpm install fuels --save
  6. Generate contract typegen files with the fuels package by pointing at the generated /out/debug/-abi.json and placing them in the /ui/src/contract-typegen directory

The contract that I have is a simple counter. It has a single storage variable which is incremented and decremented by 1 when the function is called. It has a couple more functions but I doubt they matter. I think you need to import the wallet from fuels and potentially have some code to call a function from the contract in a .tsx file in order for it to trigger. Try starting the server and opening it in the browser at this point and I believe the error should pop up in the console of the browser. If not then I can create a local repository and link it here.

camsjams commented 1 year ago

This can appear within any browser environment that is using Cryptography IIRC, but most typical setups include a polyfill that makes Buffer exist.

We wouldn't want to add anything into TS-SDK to change how that works, but we should at least document and test for it.

Dhaiwat10 commented 1 year ago

Yeah, Vite is one of the few 'popular' setups that don't polyfill these things. I believe that's the reason why none of our tests are failing in #728 when run in a browser environment - it uses puppeteer which runs a headless version of Chromium under the hood.

arboleya commented 1 year ago

@Braqzen In which browser have you had this problem?

@camsjams Why do you think we should not add buffer as a dependency? I wonder if Polyfills should be required only for libraries there were not originally planned to run inside the Browser.

Braqzen commented 1 year ago

@Braqzen In which browser have you had this problem?

@camsjams Why do you think we should not add buffer as a dependency? I wonder if Polyfills should be required only for libraries there were not originally planned to run inside the Browser.

I was running things in Chromium.

camsjams commented 1 year ago

@arboleya Why do you think we should not add buffer as a dependency? I wonder if Polyfills should be required only for libraries there were not originally planned to run inside the Browser.

Mostly due to the fact that it's environment specific (browser), dependent on build tool, and easier for consumer to add then remove.

If it's solely just inside the dependencies then that's fine with me

arboleya commented 1 year ago

Probably related:

danielbate commented 10 months ago

We will need to polyfill Buffer (along with other Node environment deps) should we want to configure a complete browser testing environment, akin to the one that Vite offers. Unlike the browser-like environment that puppeteer and playwright offer. Also relates to #284.

danielbate commented 5 months ago

So for browser testing in #1630, I went down the consumer configuration route, rather than introducing the polyfill our side. Purely from the stance of trying to introduce minimal interruption to the packages themselves, and only changing the test infrastructure. On doing some digging, this is what's recommended rather than attempting to account for it on our side.

I did have success in polyfilling it our side via tsup configuration with an esbuild injection if we want to go down that route.

I could also do an example with documentation based on the vite configuration that we have used for browser testing. However I don't think I will go any further in #1630 given we now have a setup with minimal interruption.