Skip to content

Move setTimeout to sys#11746

Merged
zhengbli merged 1 commit intomicrosoft:masterfrom
zhengbli:moveSetTimeoutToSys
Oct 20, 2016
Merged

Move setTimeout to sys#11746
zhengbli merged 1 commit intomicrosoft:masterfrom
zhengbli:moveSetTimeoutToSys

Conversation

@zhengbli
Copy link
Copy Markdown
Contributor

Right now if opening tsc.ts in vscode you will see errors on the setTimeout references. This is because we assumed a global setTimeout exists for tsc, however node and lib.d.ts has different return types for setTimeout. It is not correct to assume the existence of the global setTimeout, this PR moves the implementation to sys and add feature detection.

@zhengbli zhengbli merged commit 3f234f2 into microsoft:master Oct 20, 2016
@zhengbli zhengbli deleted the moveSetTimeoutToSys branch October 20, 2016 18:30
@mihailik
Copy link
Copy Markdown
Contributor

This change affects the quality of platform abstraction.

The code inside tsc does not care about the return from setTimeout (indeed, it's declared as any). Also tsc doesn't require the rich extra parameters and fancy spread args.

Effectively you're making the abstraction layer very needy and very tied to one specific platform (node) — whilst the code using that abstraction doesn't even care. It defeats the purpose of having abstraction in the first place. Rather than provide a small easy-to-control gateway to the platform this slowly grows to encompass every feature, and in ad-hoc inconsistent way too (consider Object.create which isn't as omnipresent yet is used indiscriminately unabstracted).

And of course ChakraHost fails to allow for setTimeout too! Naturally, as this addition is an ad-hoc artificial extra way out of sync with the intention of sys interface.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants