Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix module export in type definitions #106

Merged
merged 1 commit into from
Apr 2, 2024
Merged

Fix module export in type definitions #106

merged 1 commit into from
Apr 2, 2024

Conversation

amitbeck
Copy link
Contributor

@amitbeck amitbeck commented Apr 1, 2024

TL;DR: From microsoft/TypeScript#7185 (comment):

export = and import x = require('./x') corresponds to CommonJS/AMD/etc.'s notion of module.exports:

// ./a.js
module.exports = { a: 10, b: 20 };

// ./b.js
let a = require('./a');

Therefore the correct way to export the module in its type definitions would be using export =, and importing it should be done using import mockingoose = require('mockingoose').


The module is exported using module.exports =, and the current type definitions export the module using export default:

module.exports = mockingoose;

export default mockingoose;

This prevents TS users using allowSyntheticDefaultImports: true from being able to import the package correctly:

import mockingoose from 'mockingoose';

mockingoose.resetAll();
//          ~~~~~~~~
//          Property 'resetAll' does not exist on type 'typeof import("path/to/node_modules/mockingoose/types")'. ts(2339)

The following does pass TypeScript's compiler but fails at runtime:

import mockingoose from 'mockingoose';

mockingoose.default.resetAll();
//          ^^^^^^^
//          TypeError: mockingoose_1.default.default is not a function

Even after the proposed change, a regular import won't work at runtime:

import mockingoose from 'mockingoose';
//     ^^^^^^^^^^^
//     TypeError: (0 , mockingoose_1.default) is not a function

mockingoose.resetAll();

Finally, the following does work after the proposed change, as expected as it is a CommonJS module:

// Works 🎉
import mockingoose = require('mockingoose');

mockingoose.resetAll();

Until the issue is fixed, as in the proposed change, TS users using allowSyntheticDefaultImports: true are forced to use const mockingoose = require('mockingoose') which loses all typings.

`export =` is the correct TS equivalent of `module.exports` in CommonJS
@amitbeck
Copy link
Contributor Author

amitbeck commented Apr 1, 2024

@alonronin Seems that the tests need to be updated regardless, as there's a breaking change from Mongoose

@alonronin alonronin merged commit f6fec88 into alonronin:master Apr 2, 2024
1 of 2 checks passed
@alonronin
Copy link
Owner

@alonronin Seems that the tests need to be updated regardless, as there's a breaking change from Mongoose

yep. i need to upgrade to support new mongoose. i'm planning on rewrite in typescript.

@amitbeck amitbeck deleted the patch-1 branch April 2, 2024 13:17
@amitbeck
Copy link
Contributor Author

@alonronin Are you planning on publishing a new version including this fix to NPM?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants