zambezi / caballo-vivo

Glue code for pure RxJS applications to connect with React-Router, and other operators
2 stars 7 forks source link

Use Rollup #33

Closed cristiano-belloni closed 5 years ago

cristiano-belloni commented 5 years ago

PR

This takes inspiration from https://github.com/rollup/rollup-starter-lib. It creates three bundles:

Rig

I tested this with a simple rig that is just a create-react-app application containing these lines in App.js:

import { createLocation$ } from '@zambezi/caballo-vivo/src/location'
import { stow } from '@zambezi/caballo-vivo'

With the rig, I can run the app with npm start and the tests with npm test without errors. In comparison, caballo-vivo@0.5.0 will fail the CRA default App.test.js test because npm doesn't understand imports. This proves that esm and commonjs are working.

Config

Some modules have to be configured particularly with namedExports (see https://github.com/zambezi/caballo-vivo/compare/main-module-targets...build-with-rollup?expand=1#diff-ff6e5f22a9c7e66987b19c0199636480R18 onwards). This is due, as far as I understand, to some other libraries (for example, Immutable) not exporting esm modules, because this ES library stuff is crazy and having 72 ways of bundling a module is ungodly.

Also, we have to use external to suppress the warning on dependencies that wouldn't be bundled anyways, which seems a bit pleonastic but apparently is what we're expected to do

Bundle size

I did no analysis on the bundle size. We will have to try with the appropriate tools, but I have no time at the moment.

However, I still suspect that the big bundle size is not due to the way caballo-vivo it's packaged, but to the version ranges in package.json.

I discovered that caret ranges work between subsequent minors if the major is 0, according to this. So, having Ramda (like we have) at ^0.25.0 means >=0.25.0 <0.26.0. Current Ramda is 0.26.1, so it might be that the SPA will download its latest, but caballo-vivo will bring in the latest of the 0.25.x range.

Still, I'm still not sure what range we should specify. Should we have 0.25.0 < Ramda < 1.0.0 explicitly? I think that versions are a separate rabbit hole.

cristiano-belloni commented 5 years ago

@gabrielmontagne @ollyhayes - what do you think?

gabrielmontagne commented 5 years ago

I think this is good. It's also good that we "outsource/offshore" as much as possible the accidental complexity of thinking about this bundling crap. A curated "just do this" gives us back time to work on the valuable stuff.

that being said, we still need to vivisect the results and play with versions.

back in the paleolithic, when we worked on the grid, https://github.com/zambezi/grid/blob/master/rollup.config.js, rollup gave us a nice clean bundle we never had to look back into.

ollyhayes commented 5 years ago

I've not really used rollup before so I'm unaware of it's benefits, does this give us something that we don't get in v1.0.1? Does v1.0.1 work with your CRA rig?

To me it feels more natural to transpile each module separately and provide them to the application like that, this matches what we'd hypothetically do in the future when we no longer have to transpile anything, and it's what other libraries do right?

Happy to be proven wrong on this though!

cristiano-belloni commented 5 years ago

v1.0.1 works with my CRA rig, yes. Not clear if there are advantages with rollup, apart from having an UMD build that we don't care about much. At this point, what I'd like to do is compare rollup with current versions vs rollup with less strict versions vs 1.0.1 with current versions vs 1.0.1 with less strict versions in the context of an SPA like the ob ones and analyse what happens in terms of final bundle size.

On Sat, 4 May 2019, 08:12 Olly Hayes, notifications@github.com wrote:

I've not really used rollup before so I'm unaware of it's benefits, does this give us something that we don't get in v1.0.1? Does v1.0.1 work with your CRA rig?

To me it feels more natural to transpile each module separately and provide them to the application like that, this matches what we'd hypothetically do in the future when we no longer have to transpile anything, and it's what other libraries do right?

Happy to be proven wrong on this though!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/zambezi/caballo-vivo/pull/33#issuecomment-489302232, or mute the thread https://github.com/notifications/unsubscribe-auth/AACNDOSU6VQFEKBME6M6OTLPTUZORANCNFSM4HKXJ2SA .

ollyhayes commented 5 years ago

Sounds like a good plan, it'll be interesting to know what the differences are between those four.

I think in the case of ramda, we should just always make sure this library and the app are always on the latest minor version, that's just the extra overhead of using a pre-v1.0 library I guess. Ramda can include breaking changes on every minor version bump until they get to v1.0 so maybe specifying a wide version range might lead to issues later?

Learning a lot whilst thinking about these issues, it's great.

cristiano-belloni commented 5 years ago

Yep, I guess we'll play the price of having to update ramda's version every minor change vs having a bigger bundle until ramda hits 1.0.0.

If we're right, we will be in the position to choose what tool we want to use to transpile (rollup vs straight babel) and that'll be a good position to be in.

(ps: my rig is CRA3. I hoped the new jest would just work in CRA3 just importing from source, but it doesn't. So yeah, for now we still need to transpile)

On Sat, 4 May 2019, 08:43 Olly Hayes, notifications@github.com wrote:

Sounds like a good plan, it'll be interesting to know what the differences are between those four.

I think in the case of ramda, we should just always make sure this library and the app are always on the latest minor version, that's just the extra overhead of using a pre-v1.0 library I guess. Ramda can include breaking changes on every minor version bump until they get to v1.0 so maybe specifying a wide version range might lead to issues later?

Learning a lot whilst thinking about these issues, it's great.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/zambezi/caballo-vivo/pull/33#issuecomment-489304155, or mute the thread https://github.com/notifications/unsubscribe-auth/AACNDORLKM7EA7EB34SWKHTPTU5DBANCNFSM4HKXJ2SA .

gabrielmontagne commented 5 years ago

We solved this with #29