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

Use named exports for ESM #243

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jordanbtucker
Copy link
Member

@jordanbtucker jordanbtucker commented Oct 12, 2020

This enhances support for JavaScript modules.

This adds lib/index.mjs, which re-exports lib/index.js as a default export and parse and stringify as named exports. The module field in package.json now points to this file as the entry point for import statements in versions of Node.js that support modules. test/index.mjs ensures that lib/index.js and lib/index.mjs export the same instance. This follows the guidance in the first approach in Writing dual packages while avoiding or minimizing hazards.

lib/index.js now defines a non-enumerable default property on its export to support bundlers, like webpack, and transpilers, like TypeScript.

rollup.config.js now uses lib/index.mjs as its entry point for building browser bundles for JavaScript modules environments. It now writes browser bundles to dist/json5.umd.js and dist/json5.esm.js, as well as minified versions. These are the new canonical paths for browser bundles. The legacy dist/index.* paths still exist for backward compatibility, but they will be removed in the next major version.

All files in build and lib now explicitly require lib/index.js to ensure that lib/index.mjs is not imported. All files in test now require the package root to simulate the default behavior of the version of Node.js being tested.

Fixes #240

@jordanbtucker jordanbtucker changed the title fix: use named exports for ESM Use named exports for ESM Oct 13, 2020
@ArthurKa
Copy link

But these two ways are the same...

Code_P1DoSfUmVs

mintty_YxvsVAgokO

@jordanbtucker
Copy link
Member Author

jordanbtucker commented Feb 16, 2022

@ArthurKa By default, module.exports and exports point to the same empty object. However, if you assign a value to module.exports, then exports points to an object that will not be exported.

// mod.js
exports = {a:1}
module.exports = {b:2}

console.log('exports =', exports)
console.log('module.exports =', module.exports)
// index.js
const mod = require('./mod')

console.log('mod =', mod)
node index.js

What will this output?

exports = { a: 1 }
module.exports = { b: 2 }
mod = { b: 2 }

The issue this solved is that webpack sees exports.a = 1 differently than module.exports = {a:1}.

exports.a = 1

results in

export const a = 1

where

module.exports = {a:1}

results in

const default_1 = {a:1}
export default default_1

@ArthurKa
Copy link

ArthurKa commented Feb 17, 2022

Ah. OK. I see, I see :)

This large commit enhances support for JavaScript modules.

This adds `lib/index.mjs`, which re-exports `lib/index.js` as a default
export and `parse` and `stringify` as named exports. The `module` field
in `package.json` now points to this file as the entry point for
`import` statements in versions of Node.js that support modules.
`test/index.mjs` ensures that `lib/index.js` and `lib/index.mjs` export
the same instance. This follows the guidance in the first approach in
[Writing dual packages while avoiding or minimizing hazards][guidance].

`lib/index.js` now defines a non-enumerable `default` property on its
export to support bundlers, like webpack, and transpilers, like
TypeScript.

`rollup.config.js` now uses `lib/index.mjs` as its entry point for
building browser bundles for JavaScript modules environments. It now
writes browser bundles to `dist/json5.umd.js` and `dist/json5.esm.js`,
as well as minified versions. These are the new canonical paths for
browser bundles. The legacy `dist/index.*` paths still exist for
backward compatibility, but they will be removed in the next major
version.

All files in `build` and `lib` now explicitly require `lib/index.js` to
ensure that `lib/index.mjs` is not imported. All files in `test` now
require the package root to simulate the default behavior of the version
of Node.js being tested.

[guidance]:
  https://nodejs.org/dist/latest-v16.x/docs/api/packages.html#writing-dual-packages-while-avoiding-or-minimizing-hazards
@rxliuli
Copy link

rxliuli commented May 15, 2023

So what is the reason for not merging?

@lenovouser
Copy link

@jordanbtucker could you do a alpha release of these changes? Would like to test them out, as JSON5 is currently not usable in dual CJS and ESM modules and I think this would fix it.

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

Successfully merging this pull request may close these issues.

ESM module export differs from main export
4 participants