search

CartoDB / toolkit

JS library to interact with CARTO APIs in a simple way
https://toolkit-wheat.now.sh
BSD 3-Clause "New" or "Revised" License
9 stars 3 forks source link

Change bundler to export ESM and add viz package to toolkit package #68

Closed jesusbotella closed 4 years ago

jesusbotella commented 4 years ago

This PR modifies some important things such as bundling for toolkit and visualization packages.

When changing from rollup bundling to webpack (because of Deck.gl's compatibility with rollup), we lost the ability of exporting esm and cjs modules which are useful for developers that want to import our packages within their current build systems and tree-shake the modules.

So, I have added transpilation for @carto/toolkit and @carto/toolkit-viz to esm and cjs through Babel. This will not output a bundle (as rollup did) but separated files that bundlers are able to handle.

Another important change is that I removed browser property from all modules' package.json, because it was pointing to UMD bundles. As Webpack uses that property to get external modules, it's not OK to point to UMD, but to ESM bundle. So I removed it, and now webpack should use module field from package. We should leave UMD bundle for CDN only.

As a consequence, I had to modify rollup config to not take the proper file name from package.json but from another object. And I leveraged that opportunity to update rollup dependency version.

That said, everything should be transparent and we should not have to make any changes to the imports that we already have outside the library.

VictorVelarde commented 4 years ago

How difficult would be to completely remove rollup in favour of webpack in this PR? I understand webpack is a must for viz, due to deck.gl compatibility, so I feel like we should have just one shared tool, the sooner the better... Thoughts?

jesusbotella commented 4 years ago

I avoided doing it because I preferred rollup, and I hoped that we could use it some day, but being rational it is true that we should remove it altogether.

VictorVelarde commented 4 years ago

Yes, I aggree with you. It's up to you doing it here or as a next PR, but it will be simpler for all

jesusbotella commented 4 years ago

We don't have any package-lock.json files within packages. We just have one in our root folder that is the one updated within this PR.

I changed the review suggestions and answered questions!

VictorVelarde commented 4 years ago

Maybe its presence was a problem in my local env... I tested this on top of the style PR