Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mmomtchev opened this issue Oct 5, 2023 · 3 comments
Open

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

mmomtchev opened this issue Oct 5, 2023 · 3 comments

Comments

@mmomtchev
Copy link

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
Copy link
Author

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
Copy link

pcace commented Nov 9, 2023

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants