Fix package.json files of dedupe-mixin and scoped-elements for proper module support

[jessevanassen]

We are currently in the process of switching from Jest to Vitest. Vitest uses the built-in resolution system of NodeJS, which is stricter than that of bundlers. This causes issues with the dedupe-mixin and scoped-elements packages.

What I did

scoped-elements: Add "type": "module" to package.json

When NodeJS imports a file, it determines which module system to use based on the extension: *.cjs files use CommonJS and *.mjs use ES Modules. For *.js files, it will pick a module system based on the package's "type" field, defaulting to CommonJS. See also Determining module system in the NodeJS docs.

This system works separately from the "exports" section in the package.json: if the referenced file from the exports block has a .js extension and the package.json doesn't have a "type": "module", the file will be interpreted as a CommonJS module, even if it came from the "import" condition in the "exports".

This is the problem we're running into with the scoped-elements package: because the package.json is missing "type": "module", Node imports the package as if it was a CommonJS module instead of an ES module, resulting in the following error:

SyntaxError: Named export 'ScopedElementsMixin' not found. The requested module '@open-wc/scoped-elements' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from '@open-wc/scoped-elements';
const { ScopedElementsMixin } = pkg;

By adding the "type": "module" to the package.json, Node properly considers the file as an ES module.

dedupe-mixin: Add "main" and "exports" to package.json

The package.json of dedupe-mixin only contains a "module" declaration, and no "main" or "exports". "module" is non-standard, and only used by bundlers. This results in the following warnings when trying to import the package from NodeJS:

(node:43043) [DEP0151] DeprecationWarning: No "main" or "exports" field defined in the package.json for <...>/node_modules/@open-wc/dedupe-mixin/ resolving the main entry point "index.js", imported from <...>.
Default "index" lookups for the main are deprecated for ES modules.

To remove this warning, I've added "main" and "exports" fields to the package.json.

In another pull request I read that a concern about adding an "exports" field was that it would remove the possibility to do deep imports, which would make it a breaking change.
To prevent this breaking change, I've also added "./index.js" and "./src/dedupeMixin.js" to the exports, so deep imports are still possible.

[changeset-bot]

🦋 Changeset detected

Latest commit: b18c553

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@open-wc/scoped-elements	Patch
@open-wc/dedupe-mixin	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Westbrook]

Pretty sure this will count as a breaking change.

[jessevanassen]

I don't think so. Bundlers look at the "exports" without taking the "type" into account.
NodeJS does take the "type" into account, but fails as it tries to interpret the file as a CommonJS module.

So as far as I can see (and please correct me if I'm wrong, as different systems seem to do things differently and I want to know how to cover all bases) it's a bugfix, as it doesn't restrict anything, and fixes something that currently doesn't work when importing it from NodeJS.

[thepassle]

Im not sure if this is a breaking change, I think adding type: module is more "dangerous" for tooling packages/projects than packages like this, which is runtime/client only. As Jesse also mentioned, I don't think adding this changes anything in the way the module is resolved. I think we can ship this as a minor. @Westbrook do you agree?

[Westbrook]

Not 100% sure if I'd call this a breaking change, as it surfaces all the files that were previously available. However, maybe if should be and we should remove "./src/dedupeMixin.js": "./src/dedupeMixin.js" and "./index.js"as it's all reexported through "." anyways?

[thepassle]

This is a breaking change, somebody could be importing without file extensions

[jessevanassen]

@Westbrook In #2496, it was mentioned that adding just the "." export would make it a breaking change, after which the pull request starved. That's why I added the deep exports, so it wouldn't be a breaking change.

[jessevanassen]

@thepassle I've pushed an additional commit which adds the extensionless variants to the exports as well, so extensionless deep imports will also still work.

[thepassle]

We shouldnt support extensionless imports, because it bloats import maps

[michaelwarren1106]

makes total sense. got it!

[Westbrook]

@thepassle comparing notes on this…

Agree it’s a breaking change. Do you have any issues with a breaking change here? As long as the PR is updated to be processed as such, it would seem that doing this is the right path for the broadest number of users, right?

[thepassle]

Im ok with a breaking change, its just important that we treat it as such :p I think we should probably align the package exports in the same way we do for scoped elements. The only thing im unsure about is why scoped elements has a “types.js” entry, when it already has a “types” export condition. @Westbrook do you happen to know why this is the case?

[jessevanassen]

I've removed the named exports again, and turned the changeset into a major.

I also noticed another difference with scoped-elements: dedupe-mixin didn't have a "types" entry in the package.json, which means it wouldn't work with TypeScript unless the "moduleResolution" was set to either "node16", "nodeNext" or "bundler", as it would then take the newly added "types" in the "exports" into account. I've added this as well.

[bennypowers]

this is a broader ecosystem shift. for example, ttsc and ts-patch are broken on ts 5.0. ts-patch is working on a fix and presumably ttsc as well. I bring this up as food for thought when weighing out the value of a breaking change.

Personally, I think breaking user's imports is preferable to breaking the supported typescript ranges. In other words, if we can support both TS 4.9 and TS 5, I think that's work breaking user's imports, since those import path breakages are easy to update mechanically

[jfhumann]

What is the status of this? I am blocked because of this issue.

[SkippedTurn]

Is there any progress on this? This is blocking us from moving to Vitest as well