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

Add support for consumers using Node ES Module support (e.g. type=module in package.json) #544

Merged
merged 1 commit into from Oct 31, 2022

Conversation

hjdivad
Copy link
Contributor

@hjdivad hjdivad commented Oct 18, 2022

Adds support for Ember addons that opt-in to node's ES module syntax via
type=module.

When writing to a directory whose package.json is controlled by a third
party, maximum compatibility is achieved by writing either .cjs or .mjs
files as appropriate, and not .js files.

If neither package is concerned with backwards compatibility, writing ES
modules into a .js file can be fine, but writing commonjs modules into
a .js file in a directory whose package.json is controlled by a third
party (e.g. another package) is not generally going to be
forwards-compatible.

See also ember-cli/ember-cli#10053 (comment)

Co-authored-by: Robert Jackson rjackson@linkedin.com

@hjdivad
Copy link
Contributor Author

hjdivad commented Oct 18, 2022

cc @rwjblue

@hjdivad hjdivad marked this pull request as ready for review October 18, 2022 23:26
@hjdivad
Copy link
Contributor Author

hjdivad commented Oct 18, 2022

@ef4

I'd request a review but I guess I don't have commit to this repo. Anyhoo LMK if you're generally +1, want to see any changes &c.

@ef4
Copy link
Collaborator

ef4 commented Oct 18, 2022

Looks good, but can we land the ember-cli PR so we don't need to install that branch?

Adds support for Ember addons that opt-in to node's ES module syntax via
type=module.

When writing to a directory whose package.json is controlled by a third
party, maximum compatibility is achieved by writing either `.cjs` or `.mjs`
files as appropriate, and not `.js` files.

If neither package is concerned with backwards compatibility, writing ES
modules into a `.js` file can be fine, but writing commonjs modules into
a `.js` file in a directory whose package.json is controlled by a third
party (e.g. another package) is not generally going to be
forwards-compatible

Co-authored-by: Robert Jackson <rjackson@linkedin.com>
@rwjblue
Copy link
Collaborator

rwjblue commented Oct 20, 2022

Landed ember-cli/ember-cli#10053, and updated this PR to just use ember-cli/ember-cli master.

@rwjblue rwjblue added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request bug Something isn't working labels Oct 20, 2022
@rwjblue rwjblue changed the title Add support for node type=module Add support for consumers using Node ES Module support (e.g. type=module in package.json) Oct 20, 2022
@hjdivad
Copy link
Contributor Author

hjdivad commented Oct 20, 2022

@ef4 ember-cli PR landed and this one updated; this PR should be good to merge.

LMK if you'd like to see any other changes.

@rwjblue
Copy link
Collaborator

rwjblue commented Oct 31, 2022

@ef4 - I think everything's been resolved here (the ember-cli change has landed, and is included in ember-cli@4.9.0-beta.0), anything else needed here for merge+release?

@ef4 ef4 merged commit 5741550 into embroider-build:main Oct 31, 2022
@hjdivad hjdivad deleted the hjblue/node-esm branch November 1, 2022 16:49
@ef4
Copy link
Collaborator

ef4 commented Nov 29, 2022

This is released in 2.5.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants