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

forbidden to set `this` for setTimeout #2511

Merged
merged 2 commits into from Jun 13, 2019

Conversation

4 participants
@justjavac
Copy link
Contributor

commented Jun 13, 2019

ref https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout#Explanation

There's also no option to pass a thisArg to setTimeout as there is in Array methods like forEach, reduce, etc. and as shown below, using call to set this doesn't work either.

setTimeout.call(myArray, myArray.myMethod, 2000); // error: "NS_ERROR_XPC_BAD_OP_ON_WN_PROTO: Illegal operation on WrappedNative prototype object"
setTimeout.call(myArray, myArray.myMethod, 2500, 2); // same error
@@ -226,6 +233,8 @@ export function setTimeout(
delay: number,
...args: Args
): number {
// @ts-ignore

This comment has been minimized.

Copy link
@kitsonk

kitsonk Jun 13, 2019

Contributor

What error are you ignoring and why?

This comment has been minimized.

Copy link
@justjavac

justjavac Jun 13, 2019

Author Contributor

Because I used this in a function(not a class)

This comment has been minimized.

Copy link
@thefliik

thefliik Jun 13, 2019

Contributor

Possibly a helpful tip: you can put the reasoning for a ts-ignore on the same line:

example:

// @ts-ignore : because I used this in a function (not a class)
checkThis(this);

This comment has been minimized.

Copy link
@kitsonk

kitsonk Jun 13, 2019

Contributor

Defining this in the arguments would remove this error. Under strict mode, where this cannot be inferred, you have to be explicit. It isn't an error to ignore.

function setTimer(
	this: unknown,
    cb: (...args: Args) => void,
	delay: number,
	...args: unknown[]
): number {
	checkThis(this);
	// ...
}
Show resolved Hide resolved js/timers.ts Outdated
Show resolved Hide resolved js/timers.ts Outdated
@ry

ry approved these changes Jun 13, 2019

Copy link
Collaborator

left a comment

LGTM

@ry ry merged commit 42d1024 into denoland:master Jun 13, 2019

3 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@justjavac justjavac deleted the justjavac:settimeout branch Jun 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.