Skip to content

Updated Feedback #40

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

Closed
guybedford opened this issue Mar 30, 2015 · 1 comment
Closed

Updated Feedback #40

guybedford opened this issue Mar 30, 2015 · 1 comment

Comments

@guybedford
Copy link

I've just updated our implementation to match the latest changes. Feedback below.

  • 6.3.1 Should call requestInstantiateAll to ensure optimal loading (early dependency loading)
  • 6.5.2. Reflect.Loader.install - we should probably throw here if module is not a module object.
  • 4.1.6 Can call commitInstantiated when entry.[[Instantiate]] is undefined. Advisable to merge with 4.1.5 and just call fulfillInstantiate.
  • It is possible to use the fulfillX functions at the end of each requestX function instead of repeating between the two: "Set entry.[[state]] to the max of entry.[[state]] and translate/fetch"
  • 4.2.4 depEntry not defined when checking depEntry.[[State]], worth using a manual call to ensureRegistered at the top like all the others.
  • 4.2.4 Circular references still don't have a stop condition here. Some type of extra state is needed, perhaps an INSTANTIATE_ALL that fits in after instantiate, which then gets marked as LINK within instantiateAll before hitting the circular references.
@caridy
Copy link
Contributor

caridy commented Aug 5, 2015

6.3.1 Should call requestInstantiateAll to ensure optimal loading (early dependency loading)

0650581

6.5.2. Reflect.Loader.install - we should probably throw here if module is not a module object.

need more discussion. @dherman should look into this one. It is my understanding that [[Module]] could be undefined when installing it.

4.1.6 Can call commitInstantiated when entry.[[Instantiate]] is undefined. Advisable to merge with 4.1.5 and just call fulfillInstantiate.

cosmetic, we want to keep having all the fullfill* abstract operations to be called from public api methods (e.g.: loader.prototype.*). let's keep that separation.

It is possible to use the fulfillX functions at the end of each requestX function instead of repeating between the two: "Set entry.[[state]] to the max of entry.[[state]] and translate/fetch"

same as above.

4.2.4 depEntry not defined when checking depEntry.[[State]], worth using a manual call to ensureRegistered at the top like all the others.

551d06a

4.2.4 Circular references still don't have a stop condition here. Some type of extra state is needed, perhaps an INSTANTIATE_ALL that fits in after instantiate, which then gets marked as LINK within instantiateAll before hitting the circular references.

need more discussion. @dherman should look into this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants