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

Update timers to not depend on globals #6040

Closed
KyleJune opened this issue Jun 2, 2020 · 2 comments
Closed

Update timers to not depend on globals #6040

KyleJune opened this issue Jun 2, 2020 · 2 comments
Labels
cli related to cli/ dir web related to Web APIs

Comments

@KyleJune
Copy link
Contributor

KyleJune commented Jun 2, 2020

Currently timers.ts uses Date.now() to determine if timer callbacks should be called.
Sometimes people need to fake time in their tests to verify it behaves correctly. SinonJS fakes time by temporarily replacing globalThis.Date, and the timer functions on globalThis. Currently if you replace globalThis.Date or globalThis.Date.now in deno, the callbacks for timers will not be called. In the browser setTimeout and setInterval do not use the global date object, so you can safely override those globals without disrupting timers. That is necessary to be able to create a fake date implemention that has time advancing forward on its own. The way SinonJS does it is by calling setInterval with a callback that advances the fake Date implementation forward in intervals.
I believe this could be accomplished by creating a local variable in the timers.ts file like const now = Date.now.bind(Date); then replacing all calls to Date.now() with now(). That would make it so that the timers are not effected by globalThis.Date being changed or replaced.

@kevinkassimo
Copy link
Contributor

kevinkassimo commented Jun 2, 2020

This is actually not just limited to timer, but all builtins. Deno currently just uses the globals from the environment, yet they subject to overwrites and can cause unexpected behaviors (even security related). For example, the simplest way to crash REPL is window.Object = 1.

@bartlomieju bartlomieju added cli related to cli/ dir web related to Web APIs labels Jun 7, 2020
@bartlomieju
Copy link
Member

Fixed in #6552

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir web related to Web APIs
Projects
None yet
Development

No branches or pull requests

3 participants