evanw / esbuild

An extremely fast bundler for the web
https://esbuild.github.io/
MIT License
38.13k stars 1.15k forks source link

[Bug] mainFields and conditions ignored #3166

Closed JumpLink closed 1 year ago

JumpLink commented 1 year ago

I want to build my project in cjs format and esm format. If I use cjs, I want the cjs export of my dependency to be used and if I use esm, I want the esm export to be used.

So I have modified the mainFields and conditions options accordingly, something like this for cjs:

{
  bundle: true,
  format: 'cjs',
  platform: 'node',
  mainFields: ['main'],
  conditions: ['require'],
  external: ['gi://*'],
  plugins: [],
}

the export of the dependency in the package.json looks like this:

{
  "name": "@girs/gio-2.0",
  "version": "2.76.1-3.1.0",
  "description": "GJS TypeScript type definitions for Gio-2.0, generated from library version 2.76.1",
  "type": "module",
  "module": "gio-2.0.js",
  "main": "gio-2.0.cjs",
  "exports": {
    ".": {
      "import": {
        "types": "./gio-2.0.d.ts",
        "default": "./gio-2.0.js"
      },
      "require": {
        "types": "./gio-2.0.d.cts",
        "default": "./gio-2.0.cjs"
      }
    }
  },
  ...
}

Although I use cjs, have adjusted mainFields and conditions accordingly, esbuild always resolves to gio-2.0.js and not to gio-2.0.cjs.

The content of gio-2.0.cjs:

  imports.gi.versions.Gio = '2.0'
  const Gio = imports.gi.Gio;
  module.exports = Gio;

The content of gio-2.0.js:

  import Gio from 'gi://Gio?version=2.0';
  export { Gio };
  export default Gio;

So, I should never see gi://Gio?version=2.0 in my cjs bundle, but it is:

// ../../node_modules/@girs/gio-2.0/gio-2.0.js
var import_Gio_version_2 = __toESM(require("gi://Gio?version=2.0"), 1);
var gio_2_0_default = import_Gio_version_2.default;
evanw commented 1 year ago

The require condition is automatically activated for imports that use a require call and the import condition is automatically activated for imports that use an import statement. I assume you are using an import statement, so the import condition is active.

Activating the require condition doesn't help in this case because then both import and require are active, and the package you published lists import before require (thus indicating a preference for gio-2.0.js over gio-2.0.cjs in the case when both of these conditions are active). Picking the first applicable entry in the exports map is the way the exports field is specified, so esbuild appears to be working correctly here.

The author of this package has configured it so that if you want to import gio-2.0.cjs instead of gio-2.0.js, you need to use a require() call instead of an import statement. So I recommend doing that. If you really want to you can also keep using an import statement and force esbuild to load whatever file you want with a custom esbuild plugin, but that's much more complicated than just using a require() call.

The real problem here seems to be that this package has been authored strangely. Typically import and require are supposed to select between two equivalent modules in different formats. These modules are not equivalent, which seems like a fault of the package author. Package consumers aren't supposed to need to care what format the input module is in.

JumpLink commented 1 year ago

@evanw Thank you for this very fast answer.

I am the author of this package, so I can change the exports in the package.json.

The reason why this looks so strange is that I use the CJS format to bypass ESM. I am using esbuild for a GJS project (another JavaScript runtime) and there are currently two import possibilities:

My plan is to support both, but I want to prefer ESM. If the user wants to use the global import object he can use CJS, if he want to use ESM he can use ESM. At least that was my plan.

For this I am working on a esbuild plugin, so it would be quite an option for me to integrate this into the plugin, but maybe it is enough to adapt the exports from the @girs/* package.json.

What do you think would be the cleanest solution to my problem? Simply introduce a new export?

E.G.

    "exports": {
      ".": {
        "import": {
          "types": "./glib-2.0.d.ts",
          "default": "./glib-2.0.js"
        },
        "cjs": {
          "types": "./glib-2.0.d.cts",
          "default": "./glib-2.0.cjs"
        }
      }
    },
evanw commented 1 year ago

What do you think would be the cleanest solution to my problem?

I don't know anything about GJS. But to me the cleanest solution sounds like only supporting imports.gi.packageName because it's the only one that works in all situations. So I'd recommend doing that.

If you want to enable the use of the gi://packageName syntax too (which only seems important if there's some reason that the other syntax doesn't always work) and still have a dual-format package, you could potentially use another entry point for that. So that would look like this:

{
  "module": "gio-2.0.js",
  "main": "gio-2.0.cjs",
  "exports": {
    ".": {
      "import": {
        "types": "./gio-2.0.d.ts",
        "default": "./gio-2.0.js"
      },
      "require": {
        "types": "./gio-2.0.d.cts",
        "default": "./gio-2.0.cjs"
      }
    },
    "./esm-import": {
      "import": {
        "types": "./gio-2.0-esm-import.d.ts",
        "default": "./gio-2.0-esm-import.js"
      }
    }
  }
}

So gio-2.0.js and gio-2.0.cjs would be the same code in a different format (gio-2.0.js being ESM and gio-2.0.cjs being CJS, both using imports.gi.packageName) and gio-2.0-esm-import.js would be different code that uses gi://packageName (but ESM-only). Then users could import from @girs/gio-2.0/esm-import (or whatever subpath you want) to opt in to the ESM-only import syntax.

JumpLink commented 1 year ago

Thank you very much. I think ESM will become the standard in GJS and imports.gi is only in there for backwards compatibility (and because GNOME Shell Extensions don't support ESM yet). Therefore I would actually already like to use ESM by default. But maybe I make an extra export if someone does not want to use ESM.

JumpLink commented 1 year ago

@evanw Is there a way to transform the imports to requires to fix my issue this way? Maybe using babel (e.g. babel-plugin-transform-modules-commonjs), an esbuild option or an existing esbuild plugin?

Looks like the esbuild supported option has no option to disable esm import support? Would it help to set target to node or es5?

Otherwise I am thinking about writing a plugin for it , would it already be enough to simply resolve to the exported require file on onResolve?

Maybe related

evanw commented 1 year ago

would it already be enough to simply resolve to the exported require file on onResolve?

Yes, I think so. That would basically be pretending that your package is CommonJS-only.

Another option would be to use a custom condition. Maybe something like this:

{
  "module": "gio-2.0.js",
  "main": "gio-2.0.cjs",
  "exports": {
    ".": {
      "force-commonjs": {
        "types": "./gio-2.0.d.cts",
        "default": "./gio-2.0.cjs"
      },
      "default": {
        "types": "./gio-2.0.d.ts",
        "default": "./gio-2.0.js"
      }
    }
  }
}

So if the force-commonjs condition is active, your package would import from gio-2.0.cjs, otherwise it would import from gio-2.0.js. Both of these situations would be independent of how it was imported. That wouldn't require using a plugin, but it would require users to activate the force-commonjs condition if they wanted that. One benefit of doing things this way without a plugin is that it's independent of which bundler you use (assuming the bundler supports exports conditions, which all major ones do). Or you could write an esbuild plugin that adds the force-commonjs condition for them without needing onResolve, if you want custom logic for when your condition is active.

tgfisher4 commented 9 months ago

Activating the require condition doesn't help in this case because then both import and require are active, and the package you published lists import before require (thus indicating a preference for gio-2.0.js over gio-2.0.cjs in the case when both of these conditions are active). Picking the first applicable entry in the exports map is the way the exports field is specified, so esbuild appears to be working correctly here.

@evanw Is this behavior documented anywhere? I don't see it mentioned in the Conditions section. This behavior is unexpected to me because it is usually assumed that the order of key-value pairs in a JSON object is insignificant. It would seem to me a nice quality-of-life feature if the user-specified --conditions were given preference over the package's exports order: that way a user can pick any of the exports they like without relying on the package author to order them in a specific way.

Also, the How conditions work example includes the "import" condition above the "require" condition: in practice, do you see many use cases or much benefit for this order? My first thought is that it is much more likely that someone would want to use modern syntax (import) and produce a commonjs bundle for use with legacy packages or systems, than use legacy syntax (require) and produce a esm bundle.