GeoTIFF / georaster

Wrapper around Georeferenced Rasters like GeoTIFF and soon JPG and PNG that provides a standard interface
Apache License 2.0
81 stars 32 forks source link

Overhaul build process #65

Open rowanwins opened 2 years ago

rowanwins commented 2 years ago

This PR basically overhauls the build setup for georaster

This probably still needs a bit of testing to make sure nothing is broken but 🤞 I reckon it's a pretty good start.

DanielJDufour commented 2 years ago

awesome! Having a tool like rollup meant for making multiple builds would be great! You'll have to forgive me because I haven't used rollup before, so I probably have some naive questions.

My main concern is just making sure we don't break any current implementations, so I have a few questions in mind:

Are all the current builds found in https://unpkg.com/browse/georaster@1.5.6/dist/ still there, (for situations where people are directly referencing the build files through a script tag)?

Could dist/jsbundle/georaster.bundle.js move to dist/georaster.browser.bundle.js anddist/jsbundle/georaster.bundle.min.js move to dist/georaster.browser.bundle.min.js?

Can people still require("georaster") in a CJS node environment? I just want to to make sure people don't have to use dynamic imports.

Really asking because of my lack of knowledge, but what is the "exports" key in the package.json? Is that something used by rollup or other tools?

I use GeoRaster in another project, can I still make a build with the worker inlined? (Probably a silly question, but I just want to make sure we aren't prevent other projects from creating single file builds)

Any other significant changes or implications on how people implement georaster?

If I want to require georaster in node, can I require a file directly or do I have to require the build?

Otherwise, I think we can cut a georaster@next release and test integrating with that. Can the browser/jsbundle builds have a .map file? Not a strict requirement, because it seems we don't currently have one, but a nice to have :-)

Thoughts?

Thank you! I learned a lot about rollup just looking at your code!

rowanwins commented 2 years ago

I've made a few little tweaks to the config based on your requests @DanielJDufour

Are all the current builds found in https://unpkg.com/browse/georaster@1.5.6/dist/ still there, (for situations where people are directly referencing the build files through a script tag)?

So I think you'd publish this as a new major version to prevent any accidental breakages in terms of requiring non-existant/moved files.

Can people still require("georaster") in a CJS node environment? I just want to to make sure people don't have to use dynamic imports.

Yep - so that should people ppl to dist/node/georaster.js which the main field in the package.json points to.

If I want to require georaster in node, can I require a file directly or do I have to require the build?

The default approach in node should automatically direct ppl to the node-compatible-build, but if really necessary people could still do something like require('georaster/src/index.node.js'), but I don't think it would be necessary.

... what is the "exports" key in the package.json? Is that something used by rollup or other tools?

This is a new-ish NodeJS feature for packaging. It's probably not entirely necessary here, but in theory it allows you export multiple modules (which could be handy for shipping node vs browser user modules)... now that I think about it maybe that's something to explore further...

Thank you! I learned a lot about rollup just looking at your code!

No worries. This is a more complex package because of the node vs browser differences (with cross-fetch) so it's using a few different output types. But once you get going with rollup I find it easier than webpack to understand how to configure and create differing outputs. And for simple libraries (eg this one of mine) it's really straight forward.

Two final things

  1. For having a play with how the bundle is working you should be able to checkout this branch with git, and then use npm link to see how it performs in external applications (eg a node app, vs a es6 browser app etc). Here is a nice handy tutorial on npm link. I find npm link super-handy when I'm working on multiple libraries and/or apps which depend on one another.

  2. I also need to do is look at whether we need to pass things through babel or not. From my quick scan of the code you don't have any non-standard syntax so the build process shouldn't need to go through babel, but it would be good to confirm

elliots commented 1 year ago

Thankyou for this, it removes 392(!) dependencies. I did get it to work, but only after disabling the web worker due to it not being able to find geotiff. (ReferenceError: geotiff is not defined)