Page MenuHomePhabricator

Write a scheduler singleton to execute code after some other code
Closed, ResolvedPublic8 Estimated Story Points

Description

Currently we make heavy use of setTimeout (see T116237). Sometimes we wait for a browser native event, or actually want a time delay. Most often though, we do it to work around our execution order and postpone an action until later code completes.

This is imprecise and doesn't signal intent very well. Furthermore, the postponed action may itself postpone action. If another action needs to come after this, it must use setTimeout( function () { setTimeout( ... ); } ) , and it all turns into a rat race of ever-obscurer postponements.

@DLynch and I intend to step out of this mess incrementally by writing a singleton scheduler class that works something like the following:

ve.Scheduler.prototype.call = function ( immediateAction, completionTest, subsequentAction ) {
	// - execute immediateAction
	// - execute completionTest. If it returns false, call it again in a setTimeout loop
	// - once completionTest returns true, call subsequentAction
};

It would be used like in the following example:

ve.scheduler.call( closeInspectors, inspectorsAreClosed, updateDocument );

This is better than setTimeouts because the intent of the postponement is clear, and it is robust against future changes to other parts of the code that might alter the amount of postponement required.

Later, we can also wrap setTimeout to know whether a given call has run:

ve.Scheduler.prototype.setTimeout = ... // wraps setTimeout and returns id
ve.Scheduler.prototype.clearTimeout = ... // wraps clearTimeout
ve.Scheduler.prototype.isPending = function ( id ) ... // returns true/false

Then we can know whether immediateAction only ever called setTimeout through our wrapper, and if so we can automatically detect when it has completed (it has once every setTimeout it schedules, directly or recursively, has finished). So completionTest becomes redundant, but useful as a statement of intent and for error checking.

None of this *requires* any changes to immediateAction - we can stop writing obscure code immediately without a global refactor of the current mass of setTimeout calls.

Details

Related Changes in Gerrit:

Event Timeline

dchan assigned this task to DLynch.
dchan raised the priority of this task from to Medium.
dchan updated the task description. (Show Details)
dchan added a project: VisualEditor.
dchan added subscribers: dchan, Esanders, DLynch.

Change 250904 had a related patch set uploaded (by DLynch):
Add ve.Scheduler, use it to avoid conflict in ve.ui.Tool

https://gerrit.wikimedia.org/r/250904