sindresorhus / p-timeout Goto Github PK

Problem
Jest not compatible with ESM modules.

Environment
Jest: 26.6.3
babel-jest: 26.6.3

jest.config.js

const esModules = ["p-timeout"].join("|");

module.exports = {
  transformIgnorePatterns: [`/node_modules/(?!${esModules})`],
};

.babelrc

{
  "presets": ["next/babel"],
}

Error

  Test suite failed to run

    Jest encountered an unexpected token

    This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

    By default, if Jest sees a Babel config, it will use that to transform your files, ignoring "node_modules".

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/en/ecmascript-modules for how to enable it.
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/en/configuration.html

    Details:

    /Users/jthoms/dev/@paciolan/evenue-next-server/src/node_modules/p-timeout/index.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){export class TimeoutError extends Error {
                                                                                             ^^^^^^

    SyntaxError: Unexpected token 'export'

      11 | import { promiseHandler } from "./lib/promises";
      12 | import { logger } from "./logger";
    > 13 | import pTimeout from "p-timeout";
         | ^
      14 |
      15 | const remoteComponentCacheAge = ms(process.env.REMOTE_COMPONENT_CACHE_AGE || "24 hours"); // prettier-ignore
      16 | const zookeeperUrl = process.env.ZKP_URL || "localhost:2181";

      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1350:14)
      at Object.<anonymous>

I have tried solutions from:

• https://jestjs.io/docs/ecmascript-modules
• nrwl/nx#812

My next options:

• Using an old version of this library (v4)
• Rolling my own lib

I am unsure how to get this module working with Jest and am seeking some help.

See: #3 (comment)

In the current version, although Infinity is a valid value to the milliseconds parameter,

typeof Infinity === 'number' && Infinity > 0 // true

it actually times out immediately both in browsers and Nodejs environment (tested on Chrome 76.0.3809.132 and Node v12.7.0).

You can take a look at the following snippet.

setTimeout(() => console.log("Infinity timeout"), Infinity);
setTimeout(() => console.log("Immediate timeout"));

// Infinity timeout
// Immediate timeout

Nodejs also output the following message.

(node:19336) TimeoutOverflowWarning: Infinity does not fit into a 32-bit signed integer.
Timeout duration was set to 1.

I understand that it is meaningless to use setTimeout with Infinity timeout, since the callback will never invoked if setTimeout is implemented with really "infinity" timeout.

But as p-timeout, it's a timer waiting for a promise to be resolved, and thus we expected the infinity timeout for p-timeout means that no matter how long it takes to resolve the promise, we will wait.

So I suggest if the milliseconds is Infinity, simply fallback to original promise without timer wrapped, then the behavior is as expected.

When for what ever reason the fallback function throw an erro insted of returning a promise. the promise chain is broken and the error is just thrown and totaly lost by the caller.

I know that the docs state "Do something other than rejecting with an error on timeout." but som times this is not under our controlle and it wuld be nice if p-timeout wuld guard agenst this.

const doIt = () =>  pTimeout(neverEnding(), 1000, () => {
  // silly internal bug
  throw new Error('I pooped my pants'); // this will just be thrown to no one.
});

https://developer.mozilla.org/en-US/docs/Web/API/AbortController

node: 14.16.1
p-timeout: 5.0.0

After upgrading from version 4 to 5, getting this error

Error: Must use import to load ES Module: /app/available_modules/1620488012000/p-timeout/index.js
require() of ES modules is not supported.
require() of /app/available_modules/1620488012000/p-timeout/index.js from /app/index.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename /app/available_modules/1620488012000/p-timeout/index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /app/available_modules/1620488012000/p-timeout/package.json.

Since the project is using require instead of import, but after adding import statement in commonJS files

import pTimeout from 'p-timeout'
^^^^^^

SyntaxError: Cannot use import statement outside a module

converting the whole project to type: module in package.json will require all the files to be converted

So is there any other way to import the new version or should stick I for version 4 only, also what about future fixes which gets pushed on version 5?

Hello. I am refactoring an existing codebase that uses a testing utility called sinon.useFakeTimers(). Calling this method basically replaces functions like setTimeout and clearTimeout in the global scope. I don't like this, but it's unfeasible to change this at this point.

The problem is: to work around another unrelated issue, I want to wrap some of these tests using p-timeout. Since p-timeout (obviously) uses setTimeout and clearTimeout from the global scope, which got replaced by sinon.useFakeTimers(), nothing times out correctly anymore. I would like to be able to make p-timeout keep using the original versions of setTimeout and clearTimeout.

Some ideas I had:

• Option 1: A new customTimers option could be added to allow the caller to specify custom setTimeout and clearTimeout implementations (and in my case, I would provide the original implementations to it). I would use it like this:

  // my-module.js
  const { setTimeout, clearTimeout } = global;
  // ...
  await pTimeout(foo, 2000, undefined, { setTimeout, clearTimeout });

• Option 2: A new preventTimerStubbing option that, if true, would use the implementations of setTimeout and clearTimeout that were cached when the module was loaded (instead of reading the global scope directly), making it immune to future replacements of the global implementations. This has the downside that if require('p-timeout') is called too late, the replacements will already have taken place anyway. This wouldn't be a problem for my specific case though.
• Option 3: Just add const { setTimeout, clearTimeout } = global to the source code directly, making p-timeout immune to this type of mocking (unless the module is required too late, in which case there wouldn't be any difference). The downside is that it might break lots of people's codes, if they are also using sinon.useFakeTimers() and their code (or even any transitive dependency) uses p-timeout internally.

Would you be willing to accept any of these? If yes, which one? Let me know and I will open a PR. I prefer option 1, but it's your call.

Othewise, I understand - I might then just create and use a fork, or patch it with pirates.

Thank you!

I think this is the natural evolution of this module:

async function pingApi() {
	await fetch('/api');
}

await pTimeout(pingApi, {
	milliseconds: 50,
});

Or even wrapping it a-la-p-memoize:

const timedPingApi = pTimeout.function(async() {
	await fetch('/api');
}, {
	milliseconds: 50,
});

await timedPingApi()

In both cases, pTimeout can also generate a signal:

+ async function pingApi(signal) {
+ 	await fetch('/api', {signal});
}

await pTimeout(pingApi, {
	milliseconds: 50,
});

which is otherwise super verbose currently:

const controller = new AbortController()
async function pingApi(signal) {
	await fetch('/api', {signal});
}

const promise = pingApi(signal);

try {
	await pTimeout(promise, {
		milliseconds: 50,
	});
} finally {
	controller.abort()
}

Or it can also pass it around, if it already exists:

async function pingApi(signal) {
	await fetch('/api', {signal});
}

await pTimeout(pingApi, {
+	signal,
	milliseconds: 50,
});

See: sindresorhus/got#344 (comment)

When passing a promise to pTimeout that rejects, error.stack ends at p-timeout/index.js:92:13

Example 1:

import pTimeout from "p-timeout";
import { setTimeout } from "node:timers/promises";

const a = async () => {
  await setTimeout(1);
  throw new Error("oops");
};

const b = async () => {
  await pTimeout(a(), { milliseconds: 500 });
};

try {
  await b();
} catch (error) {
  console.log(error.stack);
}

Example 1 output:

Error: oops
    at a (file:///tmp/index.js:6:9)
    at async file:///tmp/node_modules/p-timeout/index.js:92:13

Example 2:

import { setTimeout } from "node:timers/promises";

const a = async () => {
  await setTimeout(1);
  throw new Error("oops");
};

const b = async () => {
  await a();
};

try {
  await b();
} catch (error) {
  console.log(error.stack);
}

Example 2 output:

Error: oops
    at a (file:///tmp/index.js:6:9)
    at async b (file:///tmp/indexjs:10:3)
    at async file:///tmp/index.js:14:3

In Example 1, these parts are missing from error.stack:

   at async b (file:///tmp/index.js:10:3)
   at async file:///tmp/index.js:14:3

Some package bundlers (SystemJS) require a main field in the package.json. It would be great to add one to this package and to the dependencies.

This is more of a question than an actual issue, and not only related to this library.

At first I started using these packages as is using webpack, like: p-defer, delay, p-timeout etc.

I assumed that all of these are isomorphic packages, which they sort of are in the sense that they work both in Node and the browser.

It seems I was mistaken however, since the latter needs an extra transpilation step.
(For not-so-modern browsers without const/let etc support anyway.)

This was not an obvious tradeoff to me as I started mass-consuming these packages all over, and I only found out that there's an issue, once I looked at a critical application using Browserstack.

My setup basically wasn't transpiling anything coming from node_module, which is a very usual setup.
At first I tried to just transpile the whole thing, but that not only increased the build time 4 times, it also introduced other bugs I just couldn't figure out.

For the time being I've whitelisted these packages, and it seems to work.
However that's obviously a very tedious and manual process, and there's no guarantee I wouldn't encounter an other one of your packages (targeted at Node4 for example, instead of the browser) as a transitive dependency without realising it.

I'm curious what you think of the situation.
I can't say it's wrong how these packages are published, since they work just as they meant to work in their target environment: Node.

However I do wonder if there's an easier solution to use your libraries in the browser, without the setup I described above, since this issue basically now stops me from using more of your packages. 😢

Even if, say, you decided to actually use an extra build step to transpile your libraries for the browser-env, I see no obvious way of serving both es5 and es6 code at the same time, separately to the browser and Node.

As a lib author who might want to publish such an isomorphic package in the future, it's a real concern for me how tooling currently doesn't seem to help the situation. 🤔

error TS2741: Property '__promisify__' is missing in type '(fn: any, milliseconds: any) => Timeout' but required in type 'typeof setTimeout'.

Similar type error with umijs/qiankun#47.

I just saw a couple node Node 18-related notes in the code. I'll add a couple more:

• Add support for cause in TimeoutError, even if unused locally
  • sindresorhus/eslint-plugin-unicorn#1342
• Use public fields instead of the constructor (this actually fixes the above issue automatically too)
  • sindresorhus/eslint-plugin-unicorn#1993
• Ensure that the type for TimeoutError matches these changes, since d.ts file is maintained manually

I consider any form of cancelable promise a bad practice. You wanna know why?
cuz promises was not designed with cancelations in mind, and you can't cancel promises such as fetch()
That's why AbortController was invented.

if you use a p-timeout or any cancelable library with a Promise.race condition together with fetch then you will still leave the request hanging

I know you are using this library in p-event also but really the new way of stop listening to an event is with AbortSignal + AddEventListener

const ctrl = new AbortController()
const { signal } = ctrl
addEventListener('message', fn, { signal })

Signal is grate cuz it can allow you to cancel multiple things at once eg when you navigate away from a single page application and want to stop listening to all events on the page, stop all xhr, and so on. They are now supported everywhere, in fs, streams fetch events and so on.

Now i know that signal isn't some form of timer but there are proposals to bring something like { signal } = AbortController.timeout(2000) - we created something like it in node-fetch: https://github.com/node-fetch/timeout-signal