fix(lib): use proper extension

[sachinraja]

Description

Library mode currently emits .js extensions for all formats, but this is not Node compliant. It should emit .mjs for es formats and .js for cjs formats with "type": "commonjs" and .js for es formats and .cjs for cjs formats with "type": "module".

Additional context

The current default es extension will break in Vitest and will need to be inlined.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[sachinraja]

This only checks for es format now, but it could check if the format includes es to support #6555

[bluwy]

Nice! Just noticed this quirk in the docs today as well. For detecting extensions, I think we should base on type: module from package.json instead, and choose the correct extension between .mjs, .js, and .cjs.

[sachinraja]

For detecting extensions, I think we should base on type: module from package.json instead, and choose the correct extension between .mjs, .js, and .cjs.

Good idea, just implemented that. I'll add a few tests for it later.

[patak-dev]

LGTM @sachinraja! Just to be safe, let's merge this one during the 2.9 beta so we give the chance to ecosystem players to test it. I don't think this should be an issue, but changing the extension of the lib output feels too big for a patch.

[bluwy]

Come to think of it, I think this is a breaking change and should be released in a major. If existing libraries rely on the default output path and upgraded Vite, the publishing would be incorrect.

[sachinraja]

I'm not sure this will actually affect a lot of people considering the docs recommended using a custom fileName function, but I would also say this is a breaking change.

[sachinraja]

Could this be released under a config option like emitNodeCompliantExtensions? I think it'd be good to get this fix out as soon as possible, there are already too many packages that don't use the correct extensions.

[bluwy]

I don't think it's necessary to add a new option for this. But at the meantime, we could make a separate PR to update the docs regarding using proper extensions too.

[patak-dev]

@sachinraja we talked about this PR with the team, and we would like to merge it but we should wait until Vite 3.0 because of the breaking change.

In the meantime, we need to review the naming because the es. and cjs. prefixes are no longer needed if we use .mjs and .cjs

[sachinraja]

Sounds good. I think removing the format prefixes for esm and cjs and would be good, if everyone agrees.

[mangs]

This is great news! I've been using a custom Rollup config to workaround this for a frontend project until now, so I'm curious can this be exposed for browser-based projects too (<script type="module">)?

I prefer it for clarity purposes and it allows for easier use in isomorphic projects (Node.js and in the browser), but there is apparently the downside of MIME type support by legacy web servers (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules#aside_%E2%80%94_.mjs_versus_.js). I'd love this to be the default behavior, but it may be a bad choice to do so for those unaware of the potential broken rendering due to missing MIME type for .mjs files. Then again, the popularity of this project could push the ecosystem to evolve to support it; I'm open to either. What are your thoughts? I'm sure I'm not alone in this.

And of course thank you for making such a great tool. 😀

[sachinraja]

@mangs I'm not sure if they've discussed that already but I think it would be better to create a separate issue for that.

[mangs]

OK I will, thank you for the feedback

[mangs]

Filed an issue here: #7963