memoize: mem intellisense broken for JS
This issue matches PR #30. I think it’s better to have a discussion here.
I believe this issue applies to all your packages that recently added types via d.ts files. I assume that the purpose was to improve intellisense. Right now, it works fine for Typescript users who have --esModuleInterop turned on, but for normal JS use it’s broken:
const mem = require('./mem')
isn’t callable, even though the primary export of this package is a function.
Instead, people have to change their code to
const mem = require('./mem').default
which seems like it goes against the normal commonjs usage.
The d.ts file seems like it exists solely to provide better intellisense, and it consists solely of typescript syntax, so I think using the typescript syntax for commonjs exports make sense. That way people who use mem in editors with intellisense will not have to add .default to all their existing require('./mem') calls.
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Reactions: 2
- Comments: 34 (22 by maintainers)
Commits related to this issue
- Refactor TypeScript definition to CommonJS compatible export (#20) — committed to sindresorhus/mimic-function by sindresorhus 5 years ago
- TypeScript - Drop faking ESM default export support — committed to sindresorhus/file-type by sindresorhus 5 years ago
- Only use a CommonJS export for the TypeScript definition Reason: https://github.com/sindresorhus/mem/issues/31 — committed to sindresorhus/on-change by sindresorhus 4 years ago
There are 2 options:
If your default export is a class the whole definition becomes much more awkward:
As I said, if both consumer and producer are esm, it’d work fine, but if
memis still cjs (eg, because a consumer’s dependency is locked to this version), the default member of the default import is what’s going to be needed to work (according to the type definitions).Lemme be blunt with y’all: DO NOT WRITE DECLARATION FILES TODAY FOR TOMORROW’S NODE ESM IMPLEMENTATION.
module.exports = ...only works in source files, it’s not a valid export in an ambient declaration.@BendingBender Alright. Let’s use the correct CommonJS-compatible syntax going forward. I will try to update existing definitions when I have time, but I could use some help with that too.
Or if you set
esModuleInteropwhich was introduced to help close that hole~@sindresorhus
Correct.
We have to use the following construct:
Yes, if they don’t set either
allowSyntheticDefaultImportsoresModuleInterop. I’ve co-authored an FAQ section on DT on this topic.The future where I don’t use
requireat all.I’m preparing for the future. I’m not interested in rewriting all my type definitions when I move my packages to ES2015 module syntax, so my type definitions assume it’s module and I also export a
.defaultto make it ES2015-module-like. I rather optimize the experience for TypeScript and ESM users. My type definitions are not intended for JS users, that’s just a bonus that happens to work.I don’t see the problem with having to use
.default. You have to do that already if you use any package compiled with Babel from ESM to CommonJS.