Skip to content

Conversation

@AlCalzone
Copy link

While migrating a project that depends on this package to ESM, I noticed that the type definitions are wrong. They try to make the package look like it contains ESM definitions, but this doesn't actually work, as can be tested with https://arethetypeswrong.github.io/

Removing the incorrect definitions lets TypeScript detect what this package really is: CommonJS and nothing more.

Output of arethetypeswrong before this change:

@serialport/bindings-interface v1.2.2

Build tools:
- [email protected]
- [email protected]
- [email protected]
- @microsoft/[email protected]

❌ Import resolved to JavaScript files, but no type declarations were found. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/UntypedResolution.md


┌───────────────────┬──────────────────────────────────┐
│                   │ "@serialport/bindings-interface" │
├───────────────────┼──────────────────────────────────┤
│ node10            │ 🟢                               │
├───────────────────┼──────────────────────────────────┤
│ node16 (from CJS) │ 🟢 (CJS)                         │
├───────────────────┼──────────────────────────────────┤
│ node16 (from ESM) │ ❌ No types                      │
├───────────────────┼──────────────────────────────────┤
│ bundler           │ ❌ No types                      │
└───────────────────┴──────────────────────────────────┘

Output of arethetypeswrong after this change:

@serialport/bindings-interface v0.0.0-development

Build tools:
- [email protected]
- [email protected]
- [email protected]
- @microsoft/[email protected]

 No problems found 🌟


┌───────────────────┬──────────────────────────────────┐
│                   │ "@serialport/bindings-interface" │
├───────────────────┼──────────────────────────────────┤
│ node10            │ 🟢                               │
├───────────────────┼──────────────────────────────────┤
│ node16 (from CJS) │ 🟢 (CJS)                         │
├───────────────────┼──────────────────────────────────┤
│ node16 (from ESM) │ 🟢 (CJS)                         │
├───────────────────┼──────────────────────────────────┤
│ bundler           │ 🟢                               │
└───────────────────┴──────────────────────────────────┘

@JulianWowra
Copy link

Look likes a duplicated of #32 with dirty fix?

@AlCalzone
Copy link
Author

Not sure what you mean by dirty fix. This package is a CJS package. The .mjs file is faking ESM and is empty:
https://www.npmjs.com/package/@serialport/bindings-interface?activeTab=code

https://github.com/arethetypeswrong/arethetypeswrong.github.io has good explanations for all possible kinds of import issue, but the general guideline is: don't lie about your package format.

Both ESM and CJS code can import a CJS package just fine without the need for any tricks.

@volkmarnissen
Copy link

Both ESM and CJS code can import a CJS package just fine without the need for any tricks.

Yes, however, typescript build will fail for ESM.
Just try to build binding-cpp with a ESM based tsconfig.json

   {
{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "module": "esnext",
    "moduleResolution": "bundler",
  }
}

The same happens if you have a dependency serialport (bindings-interface) in typescript ESM based node project.

@AlCalzone
Copy link
Author

In that case it's probably better to merge your PR instead. FWIW, I'm using this exact config with an ESM based project without issues. Maybe the bundler module resolution does not like it though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants