Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Proposal for dual ESM/CommonJS packages #273

Closed
GeoffreyBooth opened this issue Feb 25, 2019 · 87 comments
Closed

Proposal for dual ESM/CommonJS packages #273

GeoffreyBooth opened this issue Feb 25, 2019 · 87 comments

Comments

@GeoffreyBooth
Copy link
Member

GeoffreyBooth commented Feb 25, 2019

@guybedford, @jkrems and I discussed the package dual-ESM/CommonJS case and we have a small proposal, based on the current ecmascript-modules implementation:

  • The package.json "main" field reverts to its prior CommonJS-only use.

  • A new field "exports" is created that takes a string like "./src/index.js". This is the ES module entry point. "exports" is to import what "main" is to require.

Notes:

  • "exports" may in the future take an object, preserving design space for the package exports proposal.

  • If "exports" points to a .js file and "type": "module" is not set, an error is thrown similar to the “type mismatch” errors (like using --type=commonjs with an .mjs file). The error would also instruct the user to add "type": "module" to package.json. The "exports" field does not imply "type": "module".

And that’s it! This should cover the case while preserving design space for future proposals, and for Node potentially switching to ESM by default someday.

@GeoffreyBooth GeoffreyBooth added modules-agenda To be discussed in a meeting cjs esm proposal labels Feb 25, 2019
@devsnek
Copy link
Member

devsnek commented Feb 25, 2019

this still feels like esm is second-class. allowing extension searching gets rid of the entire dual mode issue.

@ljharb
Copy link
Member

ljharb commented Feb 25, 2019

I would prefer not to proceed with "exports" until we have extension lookup, at which point the easy way to get dual-mode packages will be foo.js and foo.mjs, where "main" is foo

@GeoffreyBooth
Copy link
Member Author

There’s nothing about the proposal above the prevents extension searching from happening, either opt-in or by default. This proposal need not be tied to that, and I think it’s better if it isn’t. Keep in mind that package.json isn’t only for Node’s use; build tools and other platforms will need to be able to read it, and it’s a burden on them if we force them to implement Node’s extension searching algorithm in order to determine the ESM entry point. Even if we allow automatic extension searching within the Node runtime itself, for compatibility with other tools and environments it would be better if it doesn’t extend to package.json.

@devsnek
Copy link
Member

devsnek commented Feb 25, 2019

@GeoffreyBooth whether exports exists or not, they have to do searching for the main field. why not just keep it simple.

@GeoffreyBooth
Copy link
Member Author

@devsnek Tools need only do extension lookup on main to support CommonJS, which will become increasingly uncommon as it's a Node-only thing.

@jkrems
Copy link
Contributor

jkrems commented Feb 25, 2019

way to get dual-mode packages will be foo.js and foo.mjs, where "main" is foo

This seems to imply that you'd have to have both in the same directory which is really uncommon. This doesn't fit well with either compilation-to-CJS or "esm interface in root, source in lib". It would mean more empty junk files in package roots unless I'm missing something about this suggestion.

@zenparsing
Copy link

@GeoffreyBooth You did great work anayzing packages with a "module" key. I'm curious if we've done any similar analysis of previous usage of the "exports" key. Does it conflict with any existing ecosystem usage? (Apologies if this belongs in another thread.)

@GeoffreyBooth
Copy link
Member Author

@zenparsing I looked it up, and it's been a while so I don't remember the usage number offhand but basically we can claim exports. It's either completely unused or used by only a handful of public npm packages.

@MylesBorins
Copy link
Contributor

@GeoffreyBooth I believe that the last time package.exports came up the feature was discussed as something that might be useful for common.js as well. It seems like this proposal is making the assumption that the only things to be exposed by package.exports would be ESM. It seems a bit fragile if we would eventually want to support ESM, alternatively it also seems like it may fail as we introduce more goals e.g. wasm / json / etc

@devsnek
Copy link
Member

devsnek commented Feb 25, 2019

@jkrems both ways leave a patterns of doing things out. assuming i'm not unique on this planet, this proposal is more junk for some people's package.json, but less junk for people who have a src and lib directory. personally i tend to prefer solutions for people that don't use build tooling since build tooling can automagically fill configurations in and whatnot. i think this is definitely something worth discussing more in call.

@GeoffreyBooth
Copy link
Member Author

GeoffreyBooth commented Feb 25, 2019

@MylesBorins This proposes that the shorthand string value be the ESM entry point, but the verbose object form could define CommonJS values as well. That’s why exports on its own doesn’t imply "type": "module".

@jkrems
Copy link
Contributor

jkrems commented Feb 25, 2019

@MylesBorins package.exports is to import as package.main is to require. Neither really implies a format of the thing being imported but it's about which (module) loading system they are meant for. You can set main to a native .node file just like you can set exports to a .wasm file.

@devsnek
Copy link
Member

devsnek commented Feb 25, 2019

i think there's some confusion here because with this proposal package.exports is now an overloaded term. there has already been a proposal for package.exports to control which files someone can import from your package.

@robpalme
Copy link
Contributor

I like the way this sets up a clear new modern entrypoint with strict semantics and leaves "main" with the pre-existing CJS meaning.

It feels very natural to add properties to expose independent entrypoints. Overloading main would worry me - it's harder to learn the subtleties.

@jkrems
Copy link
Contributor

jkrems commented Feb 25, 2019

In past discussions we got sidetracked talking about some properties of package#exports. It was never meant as a way to control which files someone can import from your package. There was a some amount of nudging that fell out from import map compatibility. But it was never a design goal (nor was it ever true) that package#exports would have provided true encapsulation.

@jkrems
Copy link
Contributor

jkrems commented Feb 25, 2019

i think there's some confusion here because with this proposal package.exports is now an overloaded term

It's still the same proposal, we just stripped away some of the features that came from additional assumptions and requirements. This is the level 0 of the proposal: exports as a string that mirrors main but for import instead of require.

@GeoffreyBooth
Copy link
Member Author

@zenparsing There are 7 packages in the public NPM registry that use "exports". In order of popularity: memorystorage, picolog, pinf-for-nodejs, insight.renderers.default, webdb, webstore, ws.suid. The first two are the only ones with weekly downloads greater than 2 per week.

Details
[ { name: 'memorystorage',
    version: '0.11.0',
    description: 'Memory-backed implementation of the Web Storage API',
    src: 'src/memorystorage.js',
    main: 'dist/memorystorage.umd.js',
    dist:
     { umd: 'dist/memorystorage.umd.js',
       min: 'dist/memorystorage.min.js',
       map: 'dist/memorystorage.min.js.map',
       shasum: 'b064f78c6f26c65a2b0f836c815c5748c6f1f39d',
       tarball:
        'https://registry.npmjs.org/memorystorage/-/memorystorage-0.11.0.tgz' },
    exports: [ 'MemoryStorage' ],
    directories: { test: 'tests' },
    repository:
     { type: 'git',
       url: 'git+https://github.com/download/memorystorage.git' },
    keywords:
     [ 'javascript',
       'persistence',
       'persistent objects',
       'localStorage',
       'Web Storage API' ],
    author:
     { name: 'Stijn de Witt',
       email: '[email protected]',
       url: 'http://StijnDeWitt.com' },
    contributors: [ [Object] ],
    copyright:
     '©2016 by Stijn de Witt and contributors. Some rights reserved.',
    license: 'CC-BY-4.0',
    licenseUrl: 'https://creativecommons.org/licenses/by/4.0/',
    bugs: { url: 'https://github.com/download/memorystorage/issues' },
    homepage: 'http://download.github.io/memorystorage',
    devDependencies:
     { grunt: '^0.4.5',
       'grunt-contrib-uglify': '~0.6.0',
       'grunt-umd': '^2.3.3',
       'load-grunt-tasks': '~1.0.0',
       'node-qunit-phantomjs': '^1.4.0' },
    dependencies: {},
    scripts: { test: 'node ./tests/test-node.js' },
    gitHead: 'ac7b6f9d2512a6ebc105ff61a5d27a69b98dbe5c',
    _id: '[email protected]',
    _shasum: 'b064f78c6f26c65a2b0f836c815c5748c6f1f39d',
    _from: '.',
    _npmVersion: '3.10.7',
    _nodeVersion: '6.3.1',
    _npmUser: { name: 'stijndewitt', email: '[email protected]' },
    maintainers: [ [Object] ],
    _npmOperationalInternal:
     { host: 'packages-16-east.internal.npmjs.com',
       tmp:
        'tmp/memorystorage-0.11.0.tgz_1472723856728_0.5229496594984084' } },
  { name: 'picolog',
    version: '1.0.4',
    description:
     'Tiny logging helper for use in the browser, Node and Nashorn.',
    src: 'src/picolog.js',
    main: 'dist/picolog.umd.js',
    dist:
     { umd: 'dist/picolog.umd.js',
       min: 'dist/picolog.min.js',
       map: 'dist/picolog.min.js.map',
       shasum: 'a8e0b70b081e864b88b4c858bbfcb838817585d5',
       tarball: 'https://registry.npmjs.org/picolog/-/picolog-1.0.4.tgz' },
    exports: [ 'log' ],
    directories: { test: 'tests' },
    repository:
     { type: 'git',
       url: 'git+https://github.com/download/picolog.git' },
    keywords: [ 'javascript', 'logging', 'browser', 'node', 'nashorn' ],
    author:
     { name: 'Stijn de Witt',
       email: '[email protected]',
       url: 'http://StijnDeWitt.com' },
    contributors: [],
    copyright:
     'Copyright 2015 by [Stijn de Witt](http://StijnDeWitt.com). Some rights reserved.',
    license: 'CC-BY-4.0',
    licenseUrl: 'https://creativecommons.org/licenses/by/4.0/',
    bugs: { url: 'https://github.com/download/picolog/issues' },
    homepage: 'http://download.github.io/picolog',
    scripts: { test: 'node tests/test-cjs.js' },
    devDependencies:
     { grunt: '~0.4.5',
       'grunt-contrib-jshint': '~0.10.0',
       'grunt-contrib-uglify': '~0.6.0',
       'grunt-umd': '^2.3.3',
       'load-grunt-tasks': '~1.0.0',
       mocha: '^2.3.4' },
    dependencies: {},
    gitHead: 'ab001c9cb7c4102c61296d78ab6be97631515eff',
    _id: '[email protected]',
    _shasum: 'a8e0b70b081e864b88b4c858bbfcb838817585d5',
    _from: '.',
    _npmVersion: '3.5.3',
    _nodeVersion: '5.4.0',
    _npmUser: { name: 'stijndewitt', email: '[email protected]' },
    maintainers: [ [Object] ],
    _npmOperationalInternal:
     { host: 'packages-12-west.internal.npmjs.com',
       tmp: 'tmp/picolog-1.0.4.tgz_1457918567954_0.08996963105164468' } },
  { name: 'pinf-for-nodejs',
    version: '0.6.1',
    pm: 'npm',
    publish: true,
    main: 'lib/pinf.js',
    bin: { pinf: './bin/pinf' },
    dependencies:
     { babel: '^6.5.2',
       colors: '~1.1.2',
       commander: '~2.9.0',
       deepcopy: '~0.6.3',
       deepmerge: '~1.1.0',
       'fs-extra': '~0.30.0',
       jsdom: '^9.6.0',
       'pinf-config': '0.1.x',
       'pinf-it-bundler': '0.1.x',
       'pinf-it-package-insight': '0.1.x',
       'pinf-it-program-insight': '0.1.x',
       'pinf-loader-js': '0.4.x',
       'pinf-primitives-js': '0.2.x',
       'pinf-vfs': '0.1.x',
       q: '~1.4.1',
       request: '~2.75.0',
       'require.async': '~0.1.1',
       send: '~0.14.1',
       waitfor: '~0.1.3' },
    devDependencies:
     { mocha: '~3.1.0', grunt: '~1.0.1', 'grunt-mocha': '~1.0.2' },
    'require.async': { './lib/main.js': './context' },
    scripts:
     { test: 'mocha --reporter list test/*.js',
       build: './bin/pinf bundle' },
    exports: { bundles: [Object] },
    overrides:
     { './node_modules/request/node_modules/hawk/node_modules/boom': [Object],
       './node_modules/request/node_modules/hawk/node_modules/sntp': [Object],
       './node_modules/request/node_modules/hawk/node_modules/cryptiles': [Object],
       './node_modules/request/node_modules/form-data': [Object] },
    config: { 'pio.deploy.converter': [Object] },
    gitHead: 'e2cc7499ad44764114b4e940fcbfb8e586ce8dc8',
    description: '*Status: DEV*',
    _id: '[email protected]',
    _shasum: 'df633378004044b3fd47b3f0242ada3ba240ab5a',
    _from: '.',
    _npmVersion: '3.10.8',
    _nodeVersion: '5.12.0',
    _npmUser: { name: 'cadorn', email: '[email protected]' },
    dist:
     { shasum: 'df633378004044b3fd47b3f0242ada3ba240ab5a',
       tarball:
        'https://registry.npmjs.org/pinf-for-nodejs/-/pinf-for-nodejs-0.6.1.tgz' },
    maintainers: [ [Object] ],
    _npmOperationalInternal:
     { host: 'packages-16-east.internal.npmjs.com',
       tmp:
        'tmp/pinf-for-nodejs-0.6.1.tgz_1475642069540_0.5849844384938478' },
    directories: {} },
  { uid: 'https://github.com/insight/insight.renderers.default/',
    name: 'insight.renderers.default',
    main: './lib/pack-helper.js',
    version: '0.0.5',
    pm: { publish: 'npm' },
    directories: { lib: './lib' },
    label: 'Default Insight Renderers',
    description:
     'Default JavaScript renderers for the Insight Intelligence Library',
    mappings: { domplate: './node_modules/domplate' },
    dependencies: { domplate: '^0.2.1' },
    exports: { images: [Object] },
    config: { 'pio.deploy.converter': [Object] },
    _id: '[email protected]',
    _npmVersion: '5.5.1',
    _nodeVersion: '9.2.0',
    _npmUser: { name: 'cadorn', email: '[email protected]' },
    dist:
     { integrity:
        'sha512-nNE76EOWoBNE8Eg006DfNshStZmogT+Hv3/PiBJacjWbXjhRoypBm99ogPwuh/6e1c9MhTJzqrS6UzI1GFaq2A==',
       shasum: '21807ef9ba71f16fcaf96d2ec2865964516e4d4a',
       tarball:
        'https://registry.npmjs.org/insight.renderers.default/-/insight.renderers.default-0.0.5.tgz' },
    maintainers: [ [Object] ],
    _npmOperationalInternal:
     { host: 's3://npm-registry-packages',
       tmp:
        'tmp/insight.renderers.default-0.0.5.tgz_1511146563550_0.14907408924773335' } },
  { name: 'webdb',
    version: '0.5.0',
    description:
     'Client-side database that can be synched with a remote server.',
    main: 'src/webdb.js',
    dist:
     { umd: 'dist/webdb.umd.js',
       min: 'dist/webdb.min.js',
       map: 'dist/webdb.min.js.map',
       shasum: '1acbeaa70f30c830a3c105954e01899bd10bef42',
       tarball: 'https://registry.npmjs.org/webdb/-/webdb-0.5.0.tgz' },
    exports: [ 'WebDB' ],
    directories: { test: 'tests' },
    repository:
     { type: 'git',
       url: 'git+https://github.com/download/webdb.git' },
    keywords:
     [ 'javascript',
       'persistence',
       'database',
       'localStorage',
       'synchronized',
       'client-server' ],
    author:
     { name: 'Stijn de Witt',
       email: '[email protected]',
       url: 'http://StijnDeWitt.com' },
    contributors: [],
    copyright:
     'Copyright 2015 by [Stijn de Witt](http://StijnDeWitt.com). Some rights reserved.',
    license: 'CC-BY-4.0',
    licenseUrl: 'https://creativecommons.org/licenses/by/4.0/',
    bugs: { url: 'https://github.com/download/webdb/issues' },
    homepage: 'http://download.github.io/webdb',
    devDependencies:
     { grunt: '~0.4.5',
       'grunt-contrib-jshint': '~0.11.0',
       'grunt-contrib-uglify': '~0.9.0',
       'grunt-jsdoc': '~1.0.0',
       'grunt-umd': '^2.3.3',
       'load-grunt-tasks': '~3.3.0' },
    dependencies: {},
    gitHead: 'ebeff7bb4753b231b7f34c1d0b62b9d0c86e1893',
    _id: '[email protected]',
    scripts: {},
    _shasum: '1acbeaa70f30c830a3c105954e01899bd10bef42',
    _from: '.',
    _npmVersion: '2.11.3',
    _nodeVersion: '0.12.7',
    _npmUser: { name: 'stijndewitt', email: '[email protected]' },
    maintainers: [ [Object] ] },
  { name: 'webstore',
    version: '0.9.0',
    description: 'One stop shop for Web Storage API compliant persistence.',
    main: 'src/webstore.js',
    scripts: { test: 'echo "Error: no test specified" && exit 1' },
    dist:
     { dbg: 'dist/webstore.js',
       min: 'dist/webstore.min.js',
       map: 'dist/webstore.min.js.map',
       shasum: '03b4705977512131e93714605e40751fb78ff6b9',
       tarball: 'https://registry.npmjs.org/webstore/-/webstore-0.9.0.tgz' },
    exports: [ 'WebStore' ],
    directories: { test: 'tests' },
    repository:
     { type: 'git',
       url: 'git+https://github.com/download/webstore.git' },
    keywords:
     [ 'javascript',
       'persistence',
       'persistent objects',
       'localStorage',
       'Web Storage API' ],
    author:
     { name: 'Stijn de Witt',
       email: '[email protected]',
       url: 'http://StijnDeWitt.com' },
    contributors: [],
    copyright:
     'Copyright 2015 by [Stijn de Witt](http://StijnDeWitt.com). Some rights reserved.',
    license: 'CC-BY-4.0',
    licenseUrl: 'https://creativecommons.org/licenses/by/4.0/',
    bugs: { url: 'https://github.com/download/webstore/issues' },
    homepage: 'https://github.com/download/webstore#readme',
    devDependencies:
     { grunt: '^0.4.5',
       'grunt-browserify': '^4.0.0',
       'grunt-contrib-jshint': '~0.10.0',
       'grunt-contrib-uglify': '~0.6.0',
       'grunt-jsdoc': '~0.6.7',
       'load-grunt-tasks': '~1.0.0' },
    dependencies: { memorystorage: '^0.9.4' },
    gitHead: 'a4d17e6e2f9b03d638694a21659b0941dc048aac',
    _id: '[email protected]',
    _shasum: '03b4705977512131e93714605e40751fb78ff6b9',
    _from: '.',
    _npmVersion: '2.11.3',
    _nodeVersion: '0.12.7',
    _npmUser: { name: 'stijndewitt', email: '[email protected]' },
    maintainers: [ [Object] ] },
  { name: 'ws.suid',
    version: '0.10.1',
    description: 'Distributed Service-Unique IDs that are short and sweet.',
    main: 'src/suid.js',
    dist:
     { min: 'dist/suid.min.js',
       map: 'dist/suid.min.js.map',
       shasum: 'cb9e89777f8aa90d04d4d974e5021b65254a1c1d',
       tarball: 'https://registry.npmjs.org/ws.suid/-/ws.suid-0.10.1.tgz' },
    exports: [ 'Suid' ],
    repository: { type: 'git', url: 'git://github.com/Download/suid.git' },
    author:
     { name: 'Stijn de Witt',
       email: '[email protected]',
       url: 'http://StijnDeWitt.com' },
    contributors: [],
    copyright:
     'Copyright 2015 by [Stijn de Witt](http://StijnDeWitt.com). Some rights reserved.',
    license: 'CC-BY-4.0',
    licenseUrl: 'https://creativecommons.org/licenses/by/4.0/',
    homepage: 'https://download.github.io/suid',
    devDependencies:
     { grunt: '~0.4.5',
       'load-grunt-tasks': '~1.0.0',
       'grunt-contrib-jshint': '~0.10.0',
       'grunt-contrib-uglify': '~0.6.0',
       'grunt-contrib-watch': '~0.6.1',
       'grunt-notify': '~0.3.1',
       'grunt-jsdoc': '~0.6.7' },
    gitHead: '90440bc80b99994326d07daf9cfb7d99dd274166',
    bugs: { url: 'https://github.com/Download/suid/issues' },
    _id: '[email protected]',
    scripts: {},
    _shasum: 'cb9e89777f8aa90d04d4d974e5021b65254a1c1d',
    _from: '.',
    _npmVersion: '3.5.3',
    _nodeVersion: '5.4.0',
    _npmUser: { name: 'stijndewitt', email: '[email protected]' },
    maintainers: [ [Object] ],
    _npmOperationalInternal:
     { host: 'packages-12-west.internal.npmjs.com',
       tmp: 'tmp/ws.suid-0.10.1.tgz_1457346412099_0.01512380107305944' },
    directories: {} } ]

@adrianhelvik
Copy link

I don't see why we should attempt to fix a solved problem. "main": "foo.mjs". Simple.

@jkrems
Copy link
Contributor

jkrems commented Mar 15, 2019

I don't see why we should attempt to fix a solved problem. "main": "foo.mjs". Simple.

This thread is about dual mode packages (e.g. packages that ship both require and import code). Unfortunately main only allows one value and also in many cases source code wouldn't be at the top-level of the package but in a directory like lib. So it's not quite as simple. :)

@adrianhelvik
Copy link

adrianhelvik commented Mar 15, 2019

Give precedence to .mjs over .js. Still simple. ;)

@jkrems
Copy link
Contributor

jkrems commented Mar 16, 2019

and also in many cases source code wouldn't be at the top-level of the package but in a directory like lib.

Implied here: The code for import and require may be in different directories. Forcing them to live in the same directory is somewhat awkward.

@goodmind
Copy link

goodmind commented Mar 17, 2019

@jkrems react does build like this, you just publish from different directory independent from your source

@WebReflection
Copy link
Contributor

WebReflection commented Mar 22, 2019

I don't see why we should attempt to fix a solved problem. "main": "foo.mjs". Simple.

This will break every project running node that doesn't understand .mjs

The dual module is a natural migration pattern NodeJS shouldn't underestimate.

The de-facto standard to run ESM is through the module field, which is already widely adopted by the community and bundlers.

In that way, currently published dual module can still work by simply adding a type field that points to module, but the main one is still backward compatible for older version of node.

Me, and many others, write ESM Modules and transpile it as CJS so that anything consuming modules can still use either ways.

The current proposal will break dual modules, and won't be welcomed by developers that shipped these for the last 2 years.

Proposal

  • if there is a type field, and its value is module
    • if there is a module field, use it as ESM entry
    • fallback using main as currently proposed

This is only one extra, very simple, check to perform, that won't break current state of modern npm modules.

I don't think there's any valid reason to break the whole ecosystem of dual modules so please do consider this proposal, thanks.

@GeoffreyBooth
Copy link
Member Author

@demurgos Thanks for your feedback. With the implementation as things stand today, the closest one can come to a dual package is to have "main" point to the CommonJS entry point and to tell users via your README to access the ESM entry point via a deep import, e.g. import { foo } from 'pkg/esm.mjs'; (yes, deep imports also currently work, but they require full filenames with extensions). There are many members of the group eager to allow that 'pkg/esm.mjs' to be simply 'pkg' for both CommonJS and ESM. The issue is how to achieve that within the constraints of the design decisions that have been made so far.

The group has some enthusiastic supporters of extension searching and overloading "main" to allow "main" to behave differently for CommonJS and ESM, but the group also has some who are strongly opposed. Since we operate by consensus, that means that extension searching/overloading "main" aren’t options unless several people change their minds. That’s why we shipped --es-module-specifier-resolution, and why its default is explicit. There might never be a consensus to drop this flag or flip its default. I’m operating under the assumption that the current behavior is what we have to work with, as it’s safer to assume the status quo than to assume otherwise. Even if we eventually enable extension searching, there’s possibly even stronger opposition to overloading "main", as people want to not need entry points to be side-by-side in the same folder and also people want to use .js everywhere and not need a secondary extension. That’s why I’ve said above that this proposal is still relevant even if --es-module-specifier-resolution=node becomes the default.

So the political issue that we have is that 'pkg/esm.mjs' might be considered good enough for enough people in the group such that no other solution finds consensus. I think introducing a new field is less controversial than overloading "main"—it at least avoids depending on the already-controversial extension searching, and if extension searching someday gets turned on by default it can apply to the new field too. Or the new field could get removed as part of enabling extension searching. I also proposed #324 as a different proposal that aims for satisfying the need with as little unnecessary complexity as possible, to present as small a target as possible for dissension.

I would like to solve this problem. I would like to provide a better UX than 'pkg/esm.mjs'. I think the way to get there is to work within the design we have, either via this proposal or #324. I think the require of ESM proposal in nodejs/node#49450 can get added on top of either base “new field” proposal, as can an enabling of extension searching by default if that ends up happening. This new field could be changed or even removed when those later PRs get approved, if they do. But I think the way forward is to take small steps and work within the framework we’ve established, as that seems to be the most likely path to finding consensus.

@demurgos
Copy link

Thank you for your detailed reply.

The main strength of extension-based dual packages was that consumers did not have to change specifiers. This opened a path where consumers could migrate to ESM without waiting for their dependencies, and then the dependencies could update to a dual package while keeping the interface compatible. Manually appending /esm.mjs or extensions requires more coordination.

I like your user story in #324. What I am calling for is to keep deep import specifiers such as import { map } from 'rxjs/operators'; as part of the discussion. Even if most packages have a single entry point, deep imports are common too since they are simpler to tree-shake. I regret that they are deferred to import maps or pkg-exports, even if I really like the pkg-exports proposal. But I guess you're right: "the way forward is to take small steps".

@WebReflection
Copy link
Contributor

WebReflection commented Apr 30, 2019

the closest one can come to a dual package is to have "main" point to the CommonJS entry point and to tell users via your README to access the ESM entry point via a deep import

that would make the module ambiguous in case it's meant to be published as "type": "module"

With the current state that doesn't really help with dual modules, there is a not too ugly workaround that seems to work already.

The TL;DR is that you specify the type as module, but you point at the CJS entry, and you use the module field to point at the module file. Such field could point directly at esm/index.js or it could be shortcut as m.js (see the example).

package.json

{
  "type": "module",
  "main": "cjs",
  "module": "m.js"
}

folder structure

cjs/index.js + others
esm/index.js + others
m.js
package.json

m.js

export * from './esm/index.js';

// in case the default should be exported too
import $ from './esm/index.js';
export default $;

Doing this way you have the following benefits:

  • old version of node will simply use main and work via CJS
  • all bundlers will pass through the module field if present (for the time being, and until this dual packaging has been solved concretely)
  • all you have to write in your README is import stuff from 'module/m.js' in case bundlers are not used or are incapable of solving
  • this workaround is future proof, you just need to keep m.js in there so that everyone won't need to maintain their imports once they wrote the module/m.js path
  • last, but not least, you don't need to write a single .mjs extension in your whole project

edit

You can already verify/experiment this workaround via dual-packaging-test module, already in npm

import log, {name} from 'dual-packaging-test/m.js';

log(name); // will log "dual-packaging-test"

@GeoffreyBooth
Copy link
Member Author

With the current state that doesn’t really help with dual modules, there is a not too ugly workaround that seems to work already.

@WebReflection I think what we’re looking for is a good solution that works without needing bundlers or loaders. Certainly once the user is using bundlers or loaders, all rough edges can be smoothed away and the user can get whatever they want. It would be nice if the vanilla experience was good as well, if only to lessen the load on bundlers/loaders and reduce the need for them.

@demurgos Yes, I also like the look of 'pkg/deep' rather than 'pkg/deep/index.js'. I suppose if we allowed import of extensionless files, one could create a deep extensionless file that was something like export * from './deep/index.js';; or a symlink from deep to ./deep/index.js. Both of those solutions have their own issues: extensionless files are hard for IDEs to work with, and symlinks have cross-platform concerns. But they’re options, I suppose, if package path maps don’t happen. But I think path maps will happen, as there hasn’t been opposition voiced to them and they provide a new benefit that people want, namely that they define a public API for a package. They also dovetail nicely with the import maps that are coming to browsers.

So anyway, we’re getting there. The fact that at this point what we’re debating is the appearance of import specifiers and how many or few ugly characters they need to contain is, to me, a sign of success 😄

@WebReflection
Copy link
Contributor

WebReflection commented May 2, 2019

@GeoffreyBooth

I think what we’re looking for is a good solution that works without needing bundlers or loaders

not sure it's clear but my workaround doesn't need bundlers to work: it works already ... but ...

It would be nice if the vanilla experience was good as well, if only to lessen the load on bundlers/loaders and reduce the need for them.

agreed, indeed mine is a workaround due current limitations where pointing at CJS in the main when publishing a primary ESM module is a no-go for me, and it makes the type field misleading.

I've tried to combine all features (vanilla + bundlers + legacy) in one, but I'm looking forward to not needing the workaround at all, being able to specify the type too.

jaydenseric referenced this issue in mike-marcacci/fs-capacitor May 6, 2019
@GeoffreyBooth
Copy link
Member Author

Should we perhaps add to the docs the import 'pkg/module.mjs' solution as a recommendation for now? Where pkg’s "main" points to the CommonJS entry point, and pkg’s README tells people to use 'pkg/module.mjs' as the ESM entry point. (With module.mjs just an example, any path would work.)

That’s a solution for dual packages that works today and I think is likely to continue to work under any of the proposals we’re considering. If this PR is accepted or if the require of ESM PR is accepted, and/or if extension searching is enabled, under all those scenarios I think import pkg from 'pkg/module.mjs' would still work as intended, even if the later improvements make the /module.mjs part unnecessary. We would definitely include a warning that this area is still a work-in-progress and likely to change, but documenting this method sends a message that there is already at least this method for publishing both CommonJS and ESM sources in the same package, that will likely remain workable while other options hopefully improve the usability.

Two more thoughts, though probably not to add to the docs:

  • If the package wanted to release a breaking change, it could set its "main" to the ESM entry point and the CommonJS entry point could be accessed via something like require('pkg/commonjs').
  • This is a pattern for any number of potential future entry points, like import 'pkg/browser-modern.mjs' and so on. There’s an argument for this deep import style being the preferred approach, over our various proposals how Node should map the bare specifier ('pkg'), as the “deep import for each entry point” style makes explicit both that a package has multiple entry points and also which one the user wants.

@ljharb
Copy link
Member

ljharb commented May 8, 2019

I don't think we should bother trying to document a non-programmatic approach like "read the readme".

@MylesBorins
Copy link
Contributor

MylesBorins commented May 14, 2019

I would like to explicitly object to dual mode packages. I genuinely believe that the removal of dual mode due to lack of automatic extension searching is a feature not a bug. Have a single specifier mean two different things depending on what graph you load it into is a massive hazard. I will spend some time this week going through some scenarios to explain some situations that we might be creating with dual mode and ways in which it creates extremely nasty and hard to debug errors.

edit:

to clarify I meant dual mode packages sharing a single specifier @GeoffreyBooth does a good job of explaining what I meant in #273 (comment)

@weswigham
Copy link
Contributor

Have a single specifier mean two different things depending on what graph you load it into is a massive hazard.

Disagree with lack of a kind of dual mode, but agree with this sentiment, which is why I think the cjs and esm resolver need to be unified (and then cross-format calls made ok via a syncification of the resolution process as described in the other proposal).

Even if you disagree with require(esm) for whatever reason, this sentiment should be a good driver for unifying the cjs and esm resolvers (related: we probably need to reserve .wasm in cjs same as .mjs in preparation for wasm modules).

@ljharb
Copy link
Member

ljharb commented May 14, 2019

(We may want to reserve all unknown extensions in .cjs, in preparation for whatever module types we might want to add in the future)

@SMotaal
Copy link

SMotaal commented May 14, 2019

I think what is becoming more visible is the disconnect that exists between simulated interoperability and actual interoperability hidden behind some form of conventionally opinionated façade.

Even if you disagree with require(esm) for whatever reason

So maybe we can recognize that historically this has been actually doing a require(transpileToCJS(esm)) and that is probably where various concerns about failing to meet expectations for all forms of conventionally opinionated façades in a one-size de facto implementation are challenging (at least at this moment).

Have a single specifier mean two different things depending on what graph you load

So if we said a specifier can mean two things depending how the means by which it was specified — as in require(‹x⑴›) == import(‹x⑵›) where x⑴ ≃ x⑵ then imho we can say that the module record for both can be a single record; assuming of course a conforming module map keys x⑵ and use two-way mapping facilitator to adapt calls to require(…).

Yeah, that sounds like two specifiers, but they are actually one specifier meaning one this, with a separate form that adapts them to the CJS layer.

@GeoffreyBooth
Copy link
Member Author

I would like to explicitly object to dual mode packages. … Have a single specifier mean two different things depending on what graph you load it into

Specifically, I think @MylesBorins is objecting to the latter—a single specifier (like 'pkg') resolving to different files based on whether it’s referenced via import in ESM or require in CommonJS. I’m assuming he doesn’t object to the limited “dual packages” support we have in --experimental-modules now, where "main" can point to a single entry point in either ESM or CommonJS and deep paths like 'pkg/module.mjs' can point to one or more other entry points also in either ESM or CommonJS. That’s the recommended best practice I wrote about above in #273 (comment), that I suggested we add to the docs. I think if we’re not likely to move forward on any other version of dual packages, that’s all the more reason we should write this up and put it in the docs to start setting expectations that this is all that’s likely to ship.

And @weswigham, this still applies even if require of ESM happens, because people will still want to publish packages that are required into CommonJS in older versions of Node. If and when require of ESM lands, this new section of the docs can be updated appropriately.

@AlexanderOMara
Copy link

AlexanderOMara commented Jul 15, 2019

In relation to #323 (comment) I just wanted to say that while the deep import option (import 'pkg/module.mjs', documented here) does work, it's far from ideal especially from a tooling perspective.

It's not so bad if everyone who uses your package only uses it for ESM, but if you are also making a dual package which depends on another dual package you might run into issues.

Suppose you write the following, where you import something from one of those dual packages:

import {something} from 'pkg';

Now suppose you want to transpile it so other people can use both your MJS and CJS modules. Here you run into an issue, because you need to output two different files like the following:

const {something} = require('pkg');
import {something} from 'pkg/module.mjs';

Essentially, the path needs to be different based on the module system you are compiling for. This is also the case for relative imports to files in your own package (since automatic extension resolution was disabled), but that's a relatively easy problem for tooling to solve. This is more complicated, and requires knowledge of the layout of 3rd-party packages (how should it know?), and the expectation that layout will not change.

@GeoffreyBooth
Copy link
Member Author

if you are also making a dual package which depends on another dual package you might run into issues.

Can you give an example? I'm not seeing this in the example you gave above. What's different about dual depending on dual?

@AlexanderOMara
Copy link

AlexanderOMara commented Jul 15, 2019

@GeoffreyBooth You somehow need to have your ESM modules import that 3rd-party module as 'pkg/module.mjs'; while your CJS modules import it as 'pkg';.

Currently that would mean editing every file that references it after transpiling.

Ideally all one would need to do is transpile it and be done, but how can the transpiler know what to change to do that for you?

@GeoffreyBooth
Copy link
Member Author

Technically the CommonJS and ESM versions of a package are really two separate packages, that just happen to be published in the same folder tree. They don't necessarily behave identically, so it's not safe to assume that they're interchangeable.

If you're outputting a dual package, then all of your package's dependencies need to be the CommonJS ones, at least for the CommonJS version of your dual package. But your ESM version could also use all CommonJS dependencies; there's no reason it needs to be ESM all the way down. Switch to ESM dependencies when you stop publishing a CommonJS version of your package.

@AlexanderOMara
Copy link

then all of your package's dependencies need to be the CommonJS ones

Couldn't they also be dual packages? I've had no problem doing that.

Also, for tree-shaking purposes, it's ideal to have it be ESM as far down as possible.

@GeoffreyBooth
Copy link
Member Author

Couldn't they also be dual packages? I've had no problem doing that.

They'd need to be the CommonJS exports of dual packages, unless you want the CommonJS side of your dual package to only be usable in Node 12+ (where ESM is supported).

@jkrems
Copy link
Contributor

jkrems commented Jul 15, 2019

Okay, trying to recap @AlexanderOMara's concern (let me know if I'm off):

  1. I maintain package mine that depends on deep-dep.
  2. deep-dep is using the recommended flow of exposing deep-dep/cjs and deep-dep/esm.
  3. I want to support both CJS and ESM in mine.
  4. To do that, I want to write ESM code and compile it to CJS.
  5. In my ESM code, I would write something like this:
// file:///mine/lib/mine.mjs
import 'deep-dep/esm';

Problem: No compiler is smart enough right now to rewrite that to a working CJS file (if that's even reliably possible). Without manually fixing the compiled code, I will not be able to publish a package that supports both webpack's ESM-only tree-shaking and being required in node.

P.S.: I think @GeoffreyBooth's read of the situation is correct and right now the solution is "if you want to use ESM dependencies anywhere, you have to drop CJS support or write additional code manually".

@GeoffreyBooth
Copy link
Member Author

I think this deserves its own issue. Apologies if I sounded dismissive, I was only trying to explain how to do this in current --experimental-modules, not to imply that that shouldn’t change.

Off the top of my head I would think that dual packages should continue publishing their ESM entry point in "module", and CommonJS entry point in "main", and that should provide build tools with all the information they’d need to output the two variations for each version of your package.

@GeoffreyBooth
Copy link
Member Author

Closing in favor of nodejs/node#29978.

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

No branches or pull requests