Skip to content
This repository has been archived by the owner on Aug 29, 2021. It is now read-only.

Handle sync subgraph execution #69

Closed
wants to merge 4 commits into from

Conversation

guybedford
Copy link
Collaborator

This resolves #63 and #64 by implementing two architectural changes:

  1. Synchronous dependencies are always executed after asynchronous dependencies. And to achieve this, they are treated like their own top-level sync execution job. This works out because the sync subgraphs can be fully separated from an execution perspective just fine, and instead evaluated "just in time".
  2. While the module bodies execute as a promise, the stitching together of module executions no longer relies on promises but instead on execution handler listeners.

Instead of changing InnerModuleEvaluation, it is left exactly as it was. Then we create a new InnerModuleEvaluationAsync that acts specifically on async module graphs. We then separately collect "asyncDependencies" and "syncDependencies", where the new algorithm is to wait on the async dependencies, then execute the sync dependencies in order, immediately followed by synchronously executing the module itself, thus retaining the property that sync chains remain sync even when a new async dependency is added (exactly supporting the case described in #64 (comment)).

That still leaves the following case:

A.js

import './B.js';
console.log('four');

B.js

import './c.js';
import './D.js';
console.log('three');

c.js

console.log('two');

D.js

await Promise.resolve();
console.log('one');

Where in the above the logs will appear in order, with synchronous execution happening between "two" and "three", but not "three" and "four".

To get synchonous execution between "three" and "four" in the above, we need to ensure that an AsyncModule with a synchonous module body importing a synchronous module remains synchronous. And to do this, we update the dependency fulfillment to use execution task handlers over promises.

@guybedford guybedford force-pushed the sync-chains branch 2 times, most recently from 03ef888 to 256d1ab Compare May 3, 2019 07:44
@littledan
Copy link
Member

littledan commented May 3, 2019

I don't understand the example in the initial comment. I thought the ordering would be "two one three four" either way. Maybe you meant to include more different kinds of Promise work?

@tc39 tc39 deleted a comment from littledan May 3, 2019
@guybedford
Copy link
Collaborator Author

I deleted the comment re #63, those are actually necessary changes there.

I thought the ordering would be "two one three four" either way.

The change described in (1) in the description above is that we move sync dependencies of async modules to execute after the async dependencies, and just before the module itself (sync with it), whereas until now they have been executing first.

@guybedford
Copy link
Collaborator Author

I've managed to found an alternative approach to this that doesn't involve custom events, by just switching the logic around. Will reopen a new PR for that when it's ready.

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.

Editorial: Make a simple Promise-based way to evaluate a module
2 participants