Proposal for dual ESM/CommonJS packages

GeoffreyBooth commented 5 years ago

@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.

devsnek commented 5 years ago

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

ljharb commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

GeoffreyBooth commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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

```js [ { 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: 'StijnDeWitt@hotmail.com', 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: 'memorystorage@0.11.0', _shasum: 'b064f78c6f26c65a2b0f836c815c5748c6f1f39d', _from: '.', _npmVersion: '3.10.7', _nodeVersion: '6.3.1', _npmUser: { name: 'stijndewitt', email: 'StijnDeWitt@hotmail.com' }, 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: 'StijnDeWitt@hotmail.com', 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: 'picolog@1.0.4', _shasum: 'a8e0b70b081e864b88b4c858bbfcb838817585d5', _from: '.', _npmVersion: '3.5.3', _nodeVersion: '5.4.0', _npmUser: { name: 'stijndewitt', email: 'StijnDeWitt@hotmail.com' }, 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: 'pinf-for-nodejs@0.6.1', _shasum: 'df633378004044b3fd47b3f0242ada3ba240ab5a', _from: '.', _npmVersion: '3.10.8', _nodeVersion: '5.12.0', _npmUser: { name: 'cadorn', email: 'christoph@christophdorn.com' }, 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: 'insight.renderers.default@0.0.5', _npmVersion: '5.5.1', _nodeVersion: '9.2.0', _npmUser: { name: 'cadorn', email: 'christoph@christophdorn.com' }, 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: 'StijnDeWitt@hotmail.com', 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: 'webdb@0.5.0', scripts: {}, _shasum: '1acbeaa70f30c830a3c105954e01899bd10bef42', _from: '.', _npmVersion: '2.11.3', _nodeVersion: '0.12.7', _npmUser: { name: 'stijndewitt', email: 'StijnDeWitt@hotmail.com' }, 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: 'StijnDeWitt@hotmail.com', 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: 'webstore@0.9.0', _shasum: '03b4705977512131e93714605e40751fb78ff6b9', _from: '.', _npmVersion: '2.11.3', _nodeVersion: '0.12.7', _npmUser: { name: 'stijndewitt', email: 'StijnDeWitt@hotmail.com' }, 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: 'StijnDeWitt@hotmail.com', 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: 'ws.suid@0.10.1', scripts: {}, _shasum: 'cb9e89777f8aa90d04d4d974e5021b65254a1c1d', _from: '.', _npmVersion: '3.5.3', _nodeVersion: '5.4.0', _npmUser: { name: 'stijndewitt', email: 'StijnDeWitt@hotmail.com' }, maintainers: [ [Object] ], _npmOperationalInternal: { host: 'packages-12-west.internal.npmjs.com', tmp: 'tmp/ws.suid-0.10.1.tgz_1457346412099_0.01512380107305944' }, directories: {} } ] ```

adrianhelvik commented 5 years ago

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

jkrems commented 5 years ago

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 commented 5 years ago

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

jkrems commented 5 years ago

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 commented 5 years ago

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

WebReflection commented 5 years ago

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.

devsnek commented 5 years ago

we can't use the module field because a large amount of the modules in it are babel compatible modules not esm compatible modules.

WebReflection commented 5 years ago

@devsnek babel compatible modules will still need babel to work so they have practically no issues whatsoever because developers using module for babel will not need to add the type: module field in package.json.

Accordingly, I don't think that's a real issue, but if it is, then we need to come up with a way to publish dual modules.

I have packages with million downloads per months published as dual module, it'll be an absurdity to stop maintaining the CJS version because we could't find a field name that'd work for dual packaging, it'd be a community no-brainer to add such field instead, as long as dual modules are still possible.

WebReflection commented 5 years ago

A New Field Proposal

If module is off the table, maybe we could just agree on a new field that everyone could start adopting as soon as they'll start eventually adopting the type field in package.json.

Following some name I wouldn't care/mind adding in my published module to keep backward compatibility and future one:

entry-file, only if --entry-type flag will make it though - vote via 👍
type-entry, since type is already decided, hence reserved, let's use it as prefix - vote via 🎉
type-main, just to simplify the understanding of what it is (the main for the specific type) - vote via 🚀

These names are generic on purpose, so that no module or commonjs or wasm, or whatever the future reserves, will be compromised.

The logic is simple:

if there is a type field in the package.json
- if there is a type-entry field, use it as module entry point
- otherwise fallback with main or whatever current mechanism we have

This will be also valid for "type": "commonjs", so that a module can be scoped as CJS, but babel users will still be able use the module field.

Thanks for considering this, I've also put the emoji poll in place

P.S. feel free to like this comment via ❤️

GeoffreyBooth commented 5 years ago

@WebReflection For organization, do you mind opening a new issue for a new proposal? So that each thread stays tied to one topic.

WebReflection commented 5 years ago

@GeoffreyBooth done https://github.com/nodejs/modules/issues/298

However, the underlying issue is exactly the same: make dual modules possible (since these are a reality already)

GeoffreyBooth commented 5 years ago

@WebReflection Right but this issue is for the proposal in the top post, not the general feature of how to achieve dual packages. That already has its own thread in nodejs/modules#93, though we can open a new one for more general discussion of competing approaches now that we have a few proposals put forward.

See also @devsnek's proposal in the comments in https://github.com/nodejs/ecmascript-modules/pull/41 (@devsnek, do you also mind opening an issue to explain your proposal in detail?)

devsnek commented 5 years ago

@weswigham's been the one working on that, i'm just a fan of it

jaydenseric commented 5 years ago

To @GeoffreyBooth 's earlier point:

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.

This is not just a hypothetical, see this conversation regarding pikapkg.com:

https://github.com/pikapkg/analyze-npm/issues/3#issuecomment-453015538

ljharb commented 5 years ago

@jaydenseric the algorithm for CJS is already trivially implemented in every bundler and resolution package; it's not actually a burden in practice to handle this.

jaydenseric commented 5 years ago

@ljharb apparently their npm registry scraper doesn't have easy access to package files, only package.json fields.

ljharb commented 5 years ago

There’s still the possibility that node will ship with ESM having extension and directory resolution; if so, that limitation seems like a design flaw.

SMotaal commented 5 years ago

@WebReflection I think the group's hesitation around the module field stems from a good-citizen debt where many packages used this field to denote pseudo-ESM entrypoints meant for transpilers (including for the web like unpkg).

WebReflection commented 5 years ago

After re-reading various discussions, I still think this proposal would be the best to solve dual packaging while people migrate to 100% ESM in the next 8 years (speculative prediction is mine).

it's backward compatible
it doesn't require modules entry points to have a shared file in the same folder
it doesn't require a same folder index redundant workaround to point at the real folder
it's easy to explain and understand for both developers and tools
it doesn't strictly enforce dual packaging neither

The only blocker to this proposal seems to be the exports field name, which I agree might be confusing due module.exports = ... in CJS, while ESM would've used export ... instead.

Why aren't we using a different name then, and keep the proposal as it is, except for such name?

type-export, type-main, type-entry, type-index ... we've already reserved type, let's use it as a prefix for a meaningful name that won't conflict in the wild, won't confuse, but will work.

chase-moskal commented 5 years ago

how about "main-module"?

WebReflection commented 5 years ago

@chase-moskal while I'd +1 anything that works as field name, that particular one would bind the field name with the module type, so that if "type": "wasm", "main-module" wouldn't sound/feel/look right.

edit or would it? if I decouple the meaning of module from strictly ESM it might be fine after all 🤔

True that having a dual package CJS/WASM seems unlikely to happen, so that main-module would be used only for type: "module" packages, so ... maybe it should be just fine, and we could move this proposal forward?

devsnek commented 5 years ago

"module" doesn't mean that something is js source text, and neither does "esm", the only thing that does is "source text module". js defines source text modules explicitly, but you can stick anything conforming to a certain defined shape and behaviour into esm graphs, such as wasm modules, html modules, etc.

GeoffreyBooth commented 5 years ago

"module" doesn't mean that something is js source text

In other words, I think @devsnek is saying that even if your entry point was wasm it would still be in "type": "module" (though the type field would be unnecessary). Wasm would always be handled by the ESM loader just like .mjs.

It's convention for package.json fields to be camelcased like devDependencies so it should be mainModule. Another name that was considered is entrypoint.

WebReflection commented 5 years ago

@devsnek fair enough, and thanks @GeoffreyBooth for expanding.

Accordingly, would "mainModule" be a better name than "exports", keeping the rest of the proposal as is?

adrianhelvik commented 5 years ago

Sorry if I'm repetitive here, but why not just have .mjs have a higher precedence than .js. A project would have index.js and index.mjs delivering the right file for the right target. Feel free to educate me. I don't see the problem.

ljharb commented 5 years ago

@adrianhelvik i believe the concern is, when you have index.js and index.mjs, and import './index' brings in index.mjs and require('./index') brings in index.js - then you might have both ESM and CJS dependencies in your graph, thus having two conceptual copies of index in circulation at the same time.

adrianhelvik commented 5 years ago

That would be bad indeed. What if .mjs was always preferred and importing .mjs from .js using require would throw?

devsnek commented 5 years ago

then shipping mjs would be fatal to anything trying to consume your module from cjs land. i can't see many library authors going for that.

ljharb commented 5 years ago

The whole point is to be able to make a module that can be imported and required in new node, and required in old node, so that "adding ESM" can be semver-minor instead of semver-major (even if other semver-major changes are required first).

antstanley commented 5 years ago

2 cents here. GraphQL.js has been shipping ESM and CJS modules side by side since v14, so for about 8 months.

Library is written using ESM and ES2017 syntax and Flow for type checking, then using Babel to transpile to a CJS & ES2015 version alongside the ESM & ES2017 version, and both are shipped together with a single "main":"index" entry point with the .mjs and .js living side by side. If you use the --experimental-modules flag the .mjs files get precedence, but otherwise it's the same node resolution algorithm.

Most developers actually haven't noticed this at all because they carried on consuming the CJS module.

antstanley commented 5 years ago

@ljharb I'm not too sure why there is problem with having ESM and CJS modules in your dependency graph? Unless there is an expectation that every Node.js dependency is going to be rewritten, shouldn't this be expected? The v11 implementation of ESM supports this.

I would expect some performance hit by having both CJS and ESM loaded in your dependency graph, but it should still be supported.

MylesBorins commented 5 years ago

@antstanley one challenge with that approach is that we do not currently support automatic file extension resolution in the new implementation of ESM. This will break the pattern you suggested

antstanley commented 5 years ago

@MylesBorins if you require explicit extension definition in an import statement and you allow .js files to be ESM modules through "type" in package.json, then you start to paint yourself into a corner

If package.json has "type":"module" then expects all .js files in that package to be ESM, which means you have to ship .cjs files if you want to ship a CJS version and ESM version in the same package.

Which in turn means a package shipped with .js ESM and .cjs CJS modules will not be compatible with previous versions of Node as they won't understand .cjs or be able load the .js (as it expects it to be CJS not ESM).

The beauty of the way it is done in GraphQL.js is that it didn't break any downstream dependent packages using CJS and older versions of Node.

Trying to maintain strict compatibility with browsers as the default when browsers have the luxury of not having to support CommonJS is going to create too many backwards compatibility issues and corner cases in Node.

There is a working dual ESM/CommonJS package in production today with 1.2 million weekly downloads that didn't break any dependent packages when it moved to a dual ESM/CommonJS package. Why don't we just learn from that?

FWIW after reading nodejs/modules#268 it doesn't seem like dual ESM/CJS packages were considered in the decision to remove automatic extension resolution, and the potential to break backwards compatibility.

Maybe an option is to add "type":"dual" to package.json which will support automatic resolution.

So something like .. "type":"module" - No automatic type resolution, all .js files are ESM, essentially browser compatible, but not compatible with previous versions of Node.

"type":"dual" - Automatic type resolution using Node's resolution algorithm with .mjs being ESM modules with precedence over .js when using an import statement and .mjs ignored when require() is used, with an error if no module is found.

nodejs / modules

Proposal for dual ESM/CommonJS packages #273

Proposal

A New Field Proposal