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
845 stars 175 forks source link

The exports `package.json` needs a new paint job #396

Open mmomtchev opened 10 months ago

mmomtchev commented 10 months ago

Here is the exports section of the package.json :

{
  "type": "module",
  "main": "dist-node/geotiff.js",
  "module": "dist-module/geotiff.js",
  "jsdelivr": "dist-browser/geotiff.js",
  "unpkg": "dist-browser/geotiff.js",
  "exports": {
    ".": {
      "import": "./dist-module/geotiff.js",
      "require": "./dist-node/geotiff.js",
      "browser": "./dist-browser/geotiff.js"
    }
  }
}

First of all, the require version does not work anymore - it hasn't been working for some time - since quick-lru does not support CJS anymore. As it adds to the complexity, I suggest to remove it.

Second, the correct form of exports would have been (https://webpack.js.org/guides/package-exports/, https://nodejs.org/api/packages.html#packages_package_entry_points):

  "exports": {
    ".": {
      "node": {
        "import": "./dist-module/geotiff.js",
        "require": "./dist-node/geotiff.js"
      },
      "browser": "./dist-browser/geotiff.js"
    }
  }

Currently, it is not well defined what to include when importing from an ES6-capable browser bundler such as webpack - is it import or is it browser? Well, webpack picks import - which pulls the Node.js web-worker.

Then the browser bundle is a CJS (UMD?) bundle - which is correct as this is the node_modules official standard, but these days almost all browser packages use ES6 (look at OpenLayers). If you want to be perfect to a fault, you should include both:

  "exports": {
    ".": {
      "node": {
        "import": "./dist-module/geotiff.js",
        "require": "./dist-node/geotiff.js"
      },
      "browser": {
        "import": "./dist-browser/geotiff.mjs",
        "require": "./dist-browser/geotiff.cjs"
      }
    }
  }

Anyway most bundlers do not like the CJS (which is in fact an UMD?) - as there is no way to identify its named exports until it is actually loaded. So they quietly switch to the Node.js import version.

mmomtchev commented 10 months ago

Are you interested in maintaining a Node.js CJS version? In this case quick-lru must be bundled in the dist file, rollup is capable of doing it - currently the CJS is produced by tsc which does not have this capability.

If I spend a day fixing this, would you be interested in merging the PR?

pcace commented 8 months ago

Are you interested in maintaining a Node.js CJS version? In this case quick-lru must be bundled in the dist file, rollup is capable of doing it - currently the CJS is produced by tsc which does not have this capability.

If I spend a day fixing this, would you be interested in merging the PR?

hi there, i am facing the same problem - i cannot get this to run because of quick-lru (const quick_lru_1 = __importDefault(require("quick-lru"));)

did you make a pr? or a fork of a working version?

Thanks a lot for your help!

brandonfcohen1 commented 6 months ago

Hi- has there been any movement on this? I am using 2.1.2 and still having this issue