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

Most upvoted comments

There are 2 options:

// 1st
import foo = require('foo');

const fooVal: foo.Foo = ...;
foo.namedExportFn();

// 2nd
import foo = require('foo');
import {namedExportFn, Foo} from 'foo';

If your default export is a class the whole definition becomes much more awkward:

declare namespace Foo {
	// Only interfaces may be declared here. If you have a value,
	// you'll need to declare it as a static member on the class
	// or via an intersection type.
}

declare class Foo {}

export = Foo;

Why wouldn’t mem be directly callable provided @sindresorhus moves his modules to ESM, which he plans to do?

As I said, if both consumer and producer are esm, it’d work fine, but if mem is 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.

  1. How it works is still in flux. What you think works today might not tomorrow, or there may be a better way tomorrow.
  2. Because of how it’s likely to work at present, we (as in TS) will likely need to augment declaration files with some kind of “node-esm-only” marker (since it behaves very differently than the transpiled esm es syntax in a declaration file represents today, and we’ll need to flag it’s usage from commonjs). We’ll be deciding how that works once everything interop wise is decided and it’s about to unflag, which is optimistically October. So any declaration file written today will, ofc, not have that marker.
  3. Declarations that don’t accurately reflect (current) reality are dangerous. Ultimately that means incorrect intellisense results, and a drop in type coverage for a program.

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.

Only in case of very simple modules that don’t have interfaces to export and thus where you can omit the namespace declaration will the compiler enforce the import foo = require(‘foo’) kind of imports.

Or if you set esModuleInterop which was introduced to help close that hole~

@sindresorhus

1. export default mem; has to be changed to export = mem;, right?

Correct.

2. How do we do named exports using this syntax?

3. Can we continue to export interfaces? export interface CacheStorage {}. If not, what do we use?

We have to use the following construct:

declare namespace foo {
	function namedExportFn(): void;
	// importable via: import {namedExportFn} from 'foo';

	interface Foo {}
	// importable via: import {Foo} from 'foo';
}

declare function foo(fooArg: foo.Foo): void;
// importable via: import foo = require('foo');
// or with allowSyntheticDefaultImports/esModuleInterop: import foo from 'foo';

export = foo;

4. Does the user have to use the awkward import mem = require('mem') syntax if they don’t set allowSyntheticDefaultImports?

Yes, if they don’t set either allowSyntheticDefaultImports or esModuleInterop. I’ve co-authored an FAQ section on DT on this topic.

With the current designs under consideration require(“esm”) isn’t even going to work (you need to use dynamic import) - so what future is this preparing for?

The future where I don’t use require at 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 .default to 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.