automerge / automerge-classic

A JSON-like data structure (a CRDT) that can be modified concurrently by different users, and merged again automatically.
http://automerge.org/
MIT License
14.75k stars 466 forks source link

Update dependencies; switch from Browserify to Webpack #321

Closed ept closed 3 years ago

ept commented 3 years ago

I haven't updated the babel-core package dependency to the latest (8.2.2) since if I do that, yarn build throws the following exception:

ERROR in ./src/automerge.js
Module build failed (from ./node_modules/babel-loader/lib/index.js):
TypeError: /Users/martin/dev/cl/automerge/src/automerge.js: Cannot read property 'bindings' of null

I don't know if that's important. Can someone with more JS ecosystem expertise help me here?

dan-weaver commented 3 years ago

I think normally you'd leave it un minified. The reason being that whatever bundler the user is using will be minifying their entire output anyway if they desire to and then likewise, they'd build their own production source maps.

I'm not really sure what people expect from UMD builds but I don't think automerge currently supports that anyway?

nettybun commented 3 years ago

The issue is that you were mixing major version releases of Babel. Anything named "babel-plugin-" and "@babel/" doesn't work together. I fixed it here: https://github.com/heyheyhello/automerge/commit/d3a91841b142a733899a629ff12e9b8e1339106f but just realized I did it on the wrong branch πŸ™ƒπŸ™ƒ heading to bed rn but I'll port the work to update-deps tomorrow.

Unfortunately updating uuid was giving me trouble. They've switched to an ESM named-imports-only thing that doesn't seem to work with mocha? Either that or it's an issue with webpack 5. It's the only package I didn't update.

ept commented 3 years ago

@heyheyhello Thank you for looking into this! Regarding the uuid dependency, I've had that problem too. I think we can keep it at an old version for now, and deal with it separately.

Since updating the dependencies, the browser tests on Safari (run on Sauce Labs via Karma) are failing with several exceptions of the following type:

TypeError: TypedArray.of requires its this argument to subclass a TypedArray constructor
TypeError: TypedArray.from requires its this argument subclass a TypedArray constructor

Annoyingly, I don't get this error when running Karma locally on my machine. The exceptions are coming from lines of code that do something like Uint8Array.of(0x85, 0x6f, 0x4a, 0x83) or Uint8Array.from([1, 2, 3]). My guess is that maybe something is replacing Uint8Array with a shim? I believe Karma uses Browserify to bundle the JS, so maybe it is to blame? Maybe Karma should be using Webpack instead of Browserify? I have no clue what is going on here 😝

nettybun commented 3 years ago

I have no clue what is going on here 😝

Yeah I drowned in JS tooling long ago. I actually left and haven't touched webpack for years because of it haha. If you're ever wanting to simply your tech stack I'd recommend esm+typescript+esbuild. No commonJS / require('...'). Likely means moving Automerge's minimum Node version in order to support ESM, but it's worth dodging the headaches (like UUID).

ept commented 3 years ago

Hi @heyheyhello, finally got round to this. Thank you for your contribution β€”Β it's nice to simplify the build configuration and remove some dependencies!

nettybun commented 3 years ago

That's awesome it all worked out! (I'll be honest I don't remember writing the code by now πŸ˜…). From a quick read over it looks like you could remove the webpack bundle analyzer if you're not interested in its output. Thanks for the ping!

ept commented 3 years ago

Good idea, thank you! I removed it in fcee822dc60034c3350c031d27774cf4d2bb0fd2.