Skip to content

Commit

Permalink
feat: support type narrowing via isPromise and default export
Browse files Browse the repository at this point in the history
  • Loading branch information
ForbesLindesay committed Apr 25, 2020
1 parent 562a060 commit feb90a4
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 4 deletions.
4 changes: 2 additions & 2 deletions index.d.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
declare function isPromise(obj: any): boolean;
export = isPromise;

This comment has been minimized.

Copy link
@weswigham

weswigham Apr 25, 2020

FYI, as-is, this a major API breaking change to TS consumers. TS has flags (allowSyntheticDefaultImports, esModuleInterop) to allow pulling a cjs module as a default import when the runtime provides that behavior, however if you really wanted to provide a declaration file that simultaneously satisfies both entry points because you're manually patching on a default (you shouldn't, it's a bad idea, generally speaking - hell, having esm and cjs in the same package is a bad idea, generally speaking, since it's like you're cramming two packages with distinct APIs into a single namespace), you should probably do so in a backwards compatible way. In this case, that looks more like:

declare function isPromise<T, S>(obj: Promise<T> | S): obj is Promise<T>;
declare namespace isPromise {
 export { isPromise as default };
}
export = isPromise;

which more accurately reflects the new cjs module shape, and is actually backwards compatible. Better yet, you should probably just leave the type definitions on definitely typed, since these types aren't generated from the source and here the types aren't tested in any way (either on their own, or in concert with every other type definition that depends on them).

This comment has been minimized.

Copy link
@ForbesLindesay

ForbesLindesay Apr 26, 2020

Author Member

See #31 These will be released as a breaking change with tests.

declare function isPromise<T, S>(obj: Promise<T> | S): obj is Promise<T>;
export default isPromise;
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
module.exports = isPromise;
module.exports.default = isPromise;

function isPromise(obj) {
return !!obj && (typeof obj === 'object' || typeof obj === 'function') && typeof obj.then === 'function';
Expand Down
3 changes: 3 additions & 0 deletions index.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function isPromise(obj) {
return !!obj && (typeof obj === 'object' || typeof obj === 'function') && typeof obj.then === 'function';
}
9 changes: 7 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
"version": "2.1.0",
"description": "Test whether an object looks like a promises-a+ promise",
"main": "index.js",
"type": "module",
"exports": {
"import": "index.mjs",

This comment has been minimized.

Copy link
@JackStandbridge

JackStandbridge Apr 25, 2020

Trying to use a typescript template with create-react-app leads me here, looks like latest release is causing an error: Error [ERR_INVALID_PACKAGE_TARGET]: Invalid "exports" main target "index.js" defined in the package config .config/yarn/global/node_modules/is-promise/package.json. Some googling leads me to possible solution: https://www.gitmemory.com/nicolas-duvauchel

"require": "index.js"
},
"scripts": {
"test": "mocha -R spec"
},
Expand All @@ -16,7 +21,7 @@
"author": "ForbesLindesay",
"license": "MIT",
"devDependencies": {
"better-assert": "~0.1.0",
"mocha": "~1.7.4"
"better-assert": "^1.0.2",
"mocha": "^7.1.1"
}
}

0 comments on commit feb90a4

Please sign in to comment.