implementation feedback from issue #24 - first pass

[caridy]

4.1.5 1 Instantiation must be called with a loader argument.

CommitInstantiated is called from RequestInstantiate and ResolveInstantiate, which they both have access to loader. It needs loader to call Instantiation().

4.1.5 1 instance is both an argument and a local variable.

renamed to _module_

4.2.1 6 use payload instead of v for consistency?

payload

4.2.3 5 Should RequestTranslate directly, don't need RequestFetch as it is implicit in RequestTranslate

the call to RequestFetch cannot be skipped because we need payload when calling _loader_.[[Instantiate]]

4.2.4 entry intead of entry0

entry

4.2.4 Before running RequestInstantiate for dependencies, we need to do the check: depEntry = ensureRegistered(loader, depKey, metadata); if depEntry.[[state]] is "ready" then use depEntry for dep Otherwise requestInstantiate throws if the module is already defined

algo refactored to cover this case

6.5.3 Reflect.loader.uninstall(key) not Reflect.loader.uninstall(module)

key

/cc @guybedford, @dherman

[guybedford]

Looking at this again - I guess that the _payload_ comes from fetch and the _source_ from translate which would explain this signature. So it looks like the instantiate hook is called with arguments - key, originalSource, translatedSource? May be worth moving this out for further clarification for now.

[caridy]

yeah, will rollback this block, I missed that detail.

[guybedford]

Thanks @caridy for the quick review!