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

Start a vote on require(esm) markers #1621

Merged
merged 8 commits into from
Sep 20, 2024
Merged
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 52 additions & 4 deletions votes/initiateNewVote/_EDIT_ME.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

# 1. Select a subject for the vote. This can be a question addressed to the TSC
# voting members.
subject: REPLACEME
subject: Choosing a marker to customize default exports returned by `require(esm)`

# 2. You can leave the header instructions as is, or modify them if you see fit.
headerInstructions: |
Expand All @@ -24,8 +24,8 @@ headerInstructions: |
# voters express their preference for each candidates, no matter how many
# there are.
candidates:
- TODO
- TODO
- '`export { foo as "module.exports" };`'
- '`export const __cjsUnwrapDefault = true;`'

# 4. Pass the following to false if it's important to keep the candidates in the
# order you define above. Presenting candidates in a fixed order tends to
Expand All @@ -35,10 +35,58 @@ canShuffleCandidates: true
# 5. Insert here a short description of the vote objectives and link to the
# issue it was discussed on to give the full context.
footerInstructions: |
TBD
This is a follow-up to the naming bikeshed in https://github.com/nodejs/loaders/issues/221
which is for the implementation in https://github.com/nodejs/node/pull/54563
and the follow-on issue in https://github.com/nodejs/node/pull/53848
A short summary can be found in https://github.com/nodejs/node/pull/54563#pullrequestreview-2288424604

# 6. Optionally, insert a brief introduction for the vote PR, in the markdown format.
prBody: |
When a user have an existing CommonJS package like this:

```js
// foo package:
module.exports = function foo () { };

// ESM users get...
import foo from 'foo';
// CommmonJS users get..
const foo = require('foo');
```

Now that `require(esm)` is possible and they can ship ESM to their CommonJS users without breaking them, they would try something like this:

```js
// foo package
export default function foo () { };

// ESM users get...
import foo from 'foo';
// CommmonJS users get..
const foo = require('foo').default; // <--- This is now different!
```

As you can see, for CommonJS consumers this will not be exactly equivalent. Since in ESM the default exports is a property of the name space, they need to access `.default` from the result returned by
`require(esm)`. So the idea is that Node.js can take a marker from package authors to directly customize what `require(esm)` returns in this case.
To keep `const foo = require('foo')` working, the two proposed markers are:

```js
export default function foo () { };
export const __cjsUnwrapDefault = true;
```

or

```js
export default function foo () { };
export { foo as "module.exports" };
```

joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
Note that to follow up with this vote, there is [a proposal](https://github.com/nodejs/node/pull/53848) to reuse this marker for detecting whether the imported module is CJS.
There is no consensus on whether that is the right way to solve the problem yet ([see](https://github.com/nodejs/node/pull/53848#issuecomment-2244500584)), so this vote is just about
the marker naming bikeshed.
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved

The vote should stay open for 7 days, or until all TSC voting members have cast a ballot.

# 7. Optionally, choose an id that will be used for the branch name as well as
# the vote folder name. If not supplied, a UUID will be used.
Expand Down