geotiffjs / geotiff.js

geotiff.js is a small library to parse TIFF files for visualization or analysis. It is written in pure JavaScript, and is usable in both the browser and node.js applications.
https://geotiffjs.github.io/
MIT License
878 stars 183 forks source link

Improve bundling of geotiff.js with `rollup`, `esbuild` and `vite` #234

Closed manzt closed 3 years ago

manzt commented 3 years ago

TL;DR modern bundlers (vite, esbuild, rollup) can bundle geotiff from source with (near) "zero-config"

Motivation

geotiff.js can be a little tricky to bundle from source if you're not using webpack or parcel. This primarily adds/changes fields in the package.json to improve "zero-config" bundling by several modern bundlers.

Changes

1.) bump txml and pako to latest ESM versions.

2.) add a "browser" field to the package.json with false for Node builtins.

  "browser": {
    "fs": false,
    "http": false,
    "https": false,
    "url": false
  },

This field is recognized by webpack, esbuild, vite, and @rollup/plugin-node-resolve (official node resolution plugin for rollup). This field tells bundlers to import an "empty" module.

3.) adds an "alias" field to the package.json for txml/txml package export.

  "alias": {
    "txml/txml": "txml/dist/txml"
  },

Parcel v1 doesn't support ESM package exports, so upgrading txml and using the package export "txml/txml" broke npm run build in this repo. This change keeps the parcel v1 bundler happy, but the package export is kept in the source code for running tests with babel/register and modern bundlers.

4.) mark "sideEffects" as false package.json to improve tree-shaking.

  "sideEffects": false,

It works

I have created a gist for a "new" project using this branch of geotiff as a dependency. The gist is a clean config with esbuild, rollup and vite, just trying to bundle:

// index.js
import * as GeoTIFF from 'geotiff';
console.log(GeoTIFF);
console.log("it worked!");
vite # works out of the box, no plugins or config
esbuild --bundle index.js # works out of the box, no plugins or config
rollup -p 'node-resolve={browser: true}' -p commonjs index.js # needs @rollup/plugin-node-resolve & @rollup/plugin-commonjs (for lerc)

All of these tools break or need extra configuration when trying to bundle geotiff currently. It would be awesome to land these changes for more users!

manzt commented 3 years ago

EDIT: I ended up merging this branch with additional changes (oops) in a separate fork. I have removed the additional changes and rebased the branch on master. Sorry if there was any confusion!

ahocevar commented 3 years ago

This pull request probably also fixes #237.

Ghostbird commented 3 years ago

This is pretty cool. My Angular app wouldn't build after an update, and I find out that the issue's been fixed right as I encounter it 👍🏽 I assume it'll be in version 1.0.6. Any idea when that will hit npmjs.com?

Ghostbird commented 3 years ago

Just now, I see. Thanks.

Note: Version 1.04 is still shown as latest on Github in the side-bar. The 1.0.5 and 1.0.6 tags have not been marked as release I guess. It's correctly shown on NPM.

constantinius commented 3 years ago

@manzt @ahocevar There seems to be an issue with the txml aliasing: see #241

Could you help me out here, I'm lost in that topic.

ahocevar commented 3 years ago

@constantinius I did not run into this issue with Webpack v5, Parcel v2 and Rollup. But if that alias is only a workaround for Parcel v1, my opinion would be to get rid of it, since Parcel v1 is a bit outdated anyway and at least for me, has not proven reliable. But I'd be curious to hear if @manzt has a better solution.

manzt commented 3 years ago

Using the txml/txml import in the source code is valid in nodejs and modern bundlers as @ahocevar mentioned. I had to add an alias to this project because geotiff.js relies on Parcel v1 to bundle dist-node and dist-brower

This is an in-depth summary of package exports. In our case, thetxml package.json looks like this:

    "exports": {
        ".": {
            "require": "./dist/index.js",
            "import": "./dist/index.mjs"
        },
        "./txml": {
            "require": "./dist/txml.js",
            "import": "./dist/txml.mjs"
        },
        "./transformStream": {
            "require": "./dist/transformStream.js",
            "import": "./dist/transformStream.mjs"
        }
    },

meaning I can write a node script with txml as a dependency and import modules as follows without bundling:

import * as txml from 'txml';
const txml = require('txml');

import {parse} from 'txml/txml'; // PURE JS import, will play very nicely with bundlers
const {parse} = require('txml/txml');

import {tranformStream} from 'txml/transfromStream';
const {transformStream} = 'txml/transformStream';

A property with package exports is that they explicitly control what a module exports. This means that in node, the following would result in an error:

import { parse } from 'txml/dist/txml.js'; // not defined in package exports!

Since the source code for geotiff.js cannot be directly run in node, I suppose a solution would be to change txml/txml to txml/dist/txml in the source code (since I don't think most bundlers have as strict of resolution), but I'd favor the more modern syntax since folks will likely run into issues like this in the future for other packages!