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

.es.js should be renamed .mjs #16439

Closed
2 tasks done
mikestopcontinues opened this issue Apr 8, 2020 · 10 comments
Closed
2 tasks done

.es.js should be renamed .mjs #16439

mikestopcontinues opened this issue Apr 8, 2020 · 10 comments

Comments

@mikestopcontinues
Copy link

Describe the bug
This bug is described fully @ import-js/eslint-plugin-import#1717

The quick version is that eslint-plugin-import throws some errors reading .es.js files. The fix is to rename the files .mjs which officially indicates they are modules, not common-js style exports.

Expected behavior
I expect other libraries to have easy inter-op with fontawesome.

Version and implementation

    "@fortawesome/fontawesome-svg-core": "1.2.28",
    "@fortawesome/free-brands-svg-icons": "5.13.0",
    "@fortawesome/free-regular-svg-icons": "5.13.0",
    "@fortawesome/free-solid-svg-icons": "5.13.0",
    "@fortawesome/react-fontawesome": "0.1.9",

Bug report checklist

  • I have filled out as much of the above information as I can
  • [-] I have included a test case because my odds go way up that the team can fix this when I do
  • I have searched for existing issues and to the best of my knowledge this is not a duplicate
@tagliala
Copy link
Member

tagliala commented Apr 9, 2020

Hi!

Thanks for being part of the Font Awesome Community and thanks for reporting this.

Let's see if @robmadole and @mlwilkerson can help

unless their package.json has type: module, which that one doesn't.

I also read this, but I don't know if it is the case of this repo

@ljharb
Copy link

ljharb commented Apr 9, 2020

I’m other words, “.es.js” is not a thing. Published .js files should be CJS and transpiled; if you don’t want to do the best practice, then they should be .mjs files.

@robmadole
Copy link
Member

We'll start by evaluating the node.js doc on ECMAScript Modules and go from there.

@IanVS
Copy link

IanVS commented May 3, 2022

Hi, has there been any decision made on use of .js vs .mjs extensions, @robmadole?

@ljharb
Copy link

ljharb commented May 3, 2022

.mjs remains the proper file extension for ESM.

node has type:module to override .js (which is supposed to mean Script/CJS) to mean ESM, but .mjs is the proper universal extension for this. File extensions are only at the end - .es.js is just .js, and thus is not reliably distinguishable from any other .js file.

@mikestopcontinues
Copy link
Author

This is becoming a bigger impediment as time goes on. Any movement?

@robmadole
Copy link
Member

How is this a bigger impediment? Has something changed?

@mikestopcontinues
Copy link
Author

@robmadole I keep hitting on compatibility issues that require workarounds.

The most recent one was that I don't bundle @fortawesome packages in a lib that depends on them (and I shouldn't), however when referencing that library from another package for some vitest tests, the code breaks. I was forced to explicitly include @fortawesome elements in the library bundle.

It's difficult to say whether this issue comes down to vitest, tsup (bundler), or typescript package resolution. However vitest reported the issue, and the error message explicitly suggested .mjs to resolve it. Funny enough, I'd completely forgotten I opened this issue two years ago, until Google landed me here.

@ljharb
Copy link

ljharb commented Jul 18, 2022

Precisely zero tools expect .es.js - all of them expect .js with type module, or .mjs always, to mean ESM - because that's what node does, and node's decision defines what's correct for the node ecosystem.

@robmadole
Copy link
Member

All we are going to focus the conversation about this in the PR #19041

@FortAwesome FortAwesome locked and limited conversation to collaborators Jul 28, 2022
@tagliala tagliala added this to the 6.2.0 milestone Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants