oddsdk / ts-odd

An SDK for building apps with decentralized identity and storage.
https://odd.dev/
Apache License 2.0
178 stars 24 forks source link

TypeError with webpack 4.x and create-react-app #264

Open bgins opened 3 years ago

bgins commented 3 years ago

Problem

Using webnative 0.26 with the react-scripts local dev server and build results in TypeErrors.

Impact

It is difficult to use webnative 0.26 with an app generated by create-react-app.

Detail

When running react-scripts start, the local dev server fails with an error in the browser:

cra-wn-26

Running react-scripts build succeeds, but the built app does not load and fails with a Uncaught TypeError: can't convert BigInt to number error.

To Reproduce Steps to reproduce the behavior:

  1. Clone https://github.com/fission-suite/react-todomvc
  2. Upgrade it to webnative 0.26.0
  3. Test the local dev server with npm start
  4. Test the build with npm run build
matheus23 commented 3 years ago

Can you try it with current main (now that this PR is merged with a similar webpack 4.x issue)?

bgins commented 3 years ago

Tried this out on main at https://github.com/fission-suite/webnative/commit/01360d92ec4a182828d7086397dc938a8ed4e8e8 and I'm seeing the same errors.

To make sure I am testing this correctly, these were my steps:

  1. Pull the latest webnative on main
  2. nix-shell, yarn install, yarn build to build webnative
  3. In my project, npm install ../webnative (that's where webnative is relatively)
  4. Run my project as above in the original bug report

I think that's the right way to test this, but just want thorough to make sure I didn't overlook something!

matheus23 commented 3 years ago

I figured out the source of the problem: The stacktrace in the image somehow shows the wrong code for line 36. It's actually a line containing Buffer.concat(...). That function won't work if its arguments aren't buffers, which for some reason is the case with your build (but not with lots of other builds???).

I'll just remove the last remaining usage of Buffer from webnative, I think it makes sense to switch to Uint8Arrays and the uint8arrays library anyway (because that's what supported in both nodejs and the browser natively).

bgins commented 3 years ago

Nice, good find! Yeah, that makes sense to switch over to Uint8Array.

matheus23 commented 3 years ago

Thanks! Unfortunately, we can't make the switch right now without breaking our tests. jest doesn't have TextEncoder support, which is need to convert Uint8Arrays to strings. https://github.com/facebook/jest/issues/9983 Not sure how to proceed. There's a PR for this issue, but it's older than a year with no end in sight. We also can't easily switch to testEnvironment: node, because that setting is managed by jest-puppeteer.

matheus23 commented 3 years ago

More closely related discussion on this: https://github.com/achingbrain/uint8arrays/issues/17