Refactor ensuring sync parent execution

[guybedford]

This resolves #64 ensuring that the execution of synchronous modules, whose execution can only begin after deeper asynchronous dependencies have already completed, remains synchronous with their parent modules.

This ensures that adding an asynchronous dependency to a module does not change the execution of that module which could be a source of bugs during application development - adding a new and unrelated dependency should not change the timing of the modules already in the graph.

Consider the following example:

main.js

import './a.js';
import './b.js';
console.log('main');

where a.js contains:

console.log('a');
Promise.resolve().then(() => {
  console.log('a task');
});

and identically for b.js.

On execution we get "a", "b", "main" synchronously in order, followed by "a task" then "b task".

Now if we introduce a new dependency to this application which is async:

main.js

import './a.js';
import './b.js';
import './async.js';
console.log('main');

with async.js containing:

console.log('async start');
await new Promise(resolve => setImmediate(resolve));
console.log('async end');

where async.js contains a top-level await, the entire execution timing changes to instead read - "a", "b", "a task", "b task", "async start", "async end", "main".

The issue here is that introducing a dependency can now alter the interrelated timings of existing modules, which may introduce execution bugs. Typically we don't think of introducing a dependency as something that can drastically change the execution semantics of our existing code! This effect is also viral in that all modules importing this "main" file will also have microtasks now being run on their individual completions too. No module can execute synchronously with main anymore.

The approach taken in this PR is to defer the execution of synchronous dependencies to happen after the async modules have resolved, instead of immediately.

The result is that in the example above we get the following execution - "async start", "async end", "a", "b", "main", where the execution of "a", "b", "main" remains identical in semantics to the execution of "a", "b", "main" before we added the asynchronous dependency. In addition all parent importers of main will continue to execute according to their previous execution timing as well, avoiding the execution timing difference propagating up al modules in the graph. What this means is that introducing an asynchronous dependency will no longer risk drastically changing the execution semantics of our existing code from as it was before we added the asynchronous dependency.

The final approach turned out to be quite simple to implement.Circular references and error handling invariants are maintained in that cycles transition through all states together, and errors stop further execution of synchronous modules, while also throwing into both sync and async cycles as a single strongly connected component transition.

[guybedford]

Another benefit of this approach is that by delaying synchronous executions to end in order with async executions means that the upfront execution of the graph will yield to the event loop sooner. As a result this execution change should lead to less jank and upfront CPU cost.

[ljharb]

Doesn’t this mean that a polyfill in a could be defeated/prevented by (or almost as bad, not applied to) code in async? It seems very concerning to have async deps’ evaluation hoisted above the evaluation of sync deps that appear lexically before the async one.

[guybedford]

Yes, async polyfills would need to be async.

This should be weighed up against the fact that adding an async dependency will alter existing execution timing (an npm package dependency adding an async dependency in an upgrade path will change your own app executing timing, possibly introduing bugs), the cost of Rollup code behaving differently to unbundled code (the build will cause execution timing changes), , as well as the performance benefits (much less upfront CPU work since we are not firing off every execution we can, but instead yielding the event loop letting the async dependencies finish before hitting the dependent sync module CPU cost).

In addition the two bug scenarios above are not easily debuggable - and will appear to users as "random breaks". In my opinion this is the worst type of bug and far worse than the predictable polyfill bug where you can add a await 0 at the top of your polyfill to support async modules.

[ljharb]

I think in your above example, preserving relative ordering among sync deps is much less important than ensuring that lexical evaluation order be retained - imo, nothing should be evaluated prior to a.js, in any of your examples, so “a” and “b” must always be logged before anything else.

[guybedford]

Sure, I can appreciate that the relative ordering may seem surprising here despite the benefits.