google / traceur-compiler

Traceur is a JavaScript.next-to-JavaScript-of-today compiler
Apache License 2.0
8.17k stars 580 forks source link

InternalLoader .cache should be reset on each root #1051

Open johnjbarton opened 10 years ago

johnjbarton commented 10 years ago

or entries should be removed as they are successfully entered into the module store.

arv commented 10 years ago

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-finishload

The FinishLoad Abstract Operation removes a completed Load Record from all LinkSets and commits the newly loaded Module to the registry. It performs the following steps:

Does .cache map to a LinkSet?

I think the problem is that our implementation is quite different from the spec, so it is sometimes hard to map the different concepts over.

I still would like us to reimplement the loaders to be a port of the spec. I started doing this a long time ago (https://github.com/arv/traceur-compiler/compare/module-spec#diff-2) but at that time the spec was just too broken. Maybe I should go back and see if the spec is ready yet (I haven't seen the bugs getting marked as fixed though.)

Looping in @GuyBedford too.

johnjbarton commented 10 years ago

Although the spec has changed quite a bit (in terms of more details) since Guy's work, I find that his Loader implementation is close enough to the spec language to use as a guide for what the heck the spec is babbling about. https://github.com/ModuleLoader/es6-module-loader/blob/master/lib/loader.js

Our .cache is closer to [[Loads]]

[[Loads]] List of Load Record Outstanding asynchronous module load requests that have been made to this loader.

These differ if the fetch blocks and more than one root begins to traverse its dependency tree. As I understand it, each tree corresponds to a LinkSet but all of the pending loads are in the same [[Loads]]. In our code no codeUnit ever leaves the .cache so it subsumes both lists (incorrectly in that the state is not needed after the tree is complete).

(IMO the form of exacting algorithm spec used in Loader stifles innovation. The Loader spec will admit only one conforming JS implementation, even though the requirements for the Loader are no where near that constraining. And the spec comes with no tests, reinforcing the message that you must implement their way.)