Skip to content
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

[Discussion] API to define loader hooks #62

Closed
caridy opened this issue Aug 10, 2015 · 15 comments
Closed

[Discussion] API to define loader hooks #62

caridy opened this issue Aug 10, 2015 · 15 comments

Comments

@caridy
Copy link
Contributor

caridy commented Aug 10, 2015

Discussion

Purposes:

  • The solely purpose of this issue is to convey into a solid API for the 4 hooks used by loader.

Questions:

  • How to define hooks in loader objects?
  • How to customize/wrap/specialize existing hooks?

Hooks:

  • resolve
  • fetch
  • translate
  • instantiate.

The purpose of the hook system is to provides a hookable pipeline, to allow front-end packaging solutions to hook into the loading process. E.g.: if a JavaScript program wants to translate .coffee files to JavaScript on the fly, the Loader defines a "translate" hook that can be used to get the coffee source code and transpile it into javascript before instantiation process.

note: each hook has its own signature, e.g.: fetch(key) vs translate(key, payload).

Current proposal:

// fetch hook:
// - getter
var fetchHook = loader.hook('fetch');
// - setter
loader.hook('fetch', function (key) {
    // do something here...
});

note: it is confusing. the hook concept is new, it looks like an observable, but it is not. No precedent on this kind of api AKAIK.

Simple Getter and Setters:

// fetch hook:
// - getter
var fetchHook = loader.fetch;
// - setter
loader.fetch = function (key) {
    // do something here...
});

note: the problem with this approach is that one of the hooks is called resolve, and one method of loader is also called resolve(), which is an important one.

Getter and Setters:

// fetch hook:
// - getter
var fetchHook = loader.fetchHook;
// - setter
loader.fetchHook = function (key) {
    // do something here...
});

note: the problem with this approach is that there is no precedent of the concept of hooks, it might be confusing, and on top of that it is ugly IMO.

Others

  • loader.onFetch() looks like an observable/event, which is not the case, only 1 hook can be in place.
  • loader.hookFetch same as loader.fetchHook.
@matthewp
Copy link

I'm a fan of the Simple Getter and Setters approach as discussed in the subclassability issue because it will make it much easier to make custom loaders this way.

note: the problem with this approach is that one of the hooks is called resolve, and one method of loader is also called resolve(), which is an important one.

I don't think "resolve" is a good name for this hook, the proper name is "locate". It's been discussed ad nauseum about the change from normalize/locate -> resolve, but "locate" is not what was deprecated. Normalize is what was deprecated. Resolve is really just locate. It's locate with the knowledge of the parent url.

tldr; I would rename "resolve" to "locate".

@rauschma
Copy link

A few thoughts (caveat: I know very little about how the API will work):

  • Subclassing can work well for this kind of thing: Each hook is a method, customizing/using a hook means overriding the method.
  • Subclassing does not work if you want to use the same hook multiple times. Then I’d use something event-based, because that’s what most web dev people are familiar with.

If there will only ever be a limited number of hooks then I would not provide a generic solution with strings as keys. For example, a variant of “current proposal” without generic keys is:

// Get
var fetchHook = loader.hookFetch();
// Set
loader.hookFetch(function (key) {
    // do something here...
});

@matthewp
Copy link

Subclassing does not work if you want to use the same hook multiple times. Then I’d use something event-based, because that’s what most web dev people are familiar with.

Being able to call them as functions is an important part of the hooks, so if they were events they would be far less useful. One thing you do often when working on loader stuff is generate keys. I want to be able to call resolve (or whatever it is to be called) so that I can get the key for a module.

edit: Some hooks like instantiate you would almost never call this way, it's mostly resolve and to a lesser extent translate where this could be useful. But if I'm authoring a custom Loader I need to be able to call them all.

@nzakas
Copy link

nzakas commented Aug 10, 2015

I'd need some more background on the problems you're trying to solve and how this maps to those problems. Right now, I'm unsure of what I'm reading.

resolve
fetch
translate
instantiate

What do these do? And what are hooks?

@caridy
Copy link
Contributor Author

caridy commented Aug 11, 2015

@nzakas and co, let me provide more details:

The loader's "internal engine" will carry on some procedures to locate, fetch, parse and execute each module, this procedures are executed in a predictable order. But for the sake of aligning with EWM, we want to provide access to the "internal engine" of the loader, specifically, provide a mechanism to control and even alter those procedures in user-land, whether that's by using AOP or any other similar pattern.

As today, we have identified four stages during the internal life cycle that we will like to expose somehow, those are the one listed above. The question here is how to provide an API that is simple enough and powerful enough that allow users to tap into the internal procedures of the loader to add some extra functionalities for the loader.

Additionally, we want to have a single access point for each of this stages (a single hook), but users might decide to chain them (in user-land using AOP or something else) if they choose to.

@nzakas
Copy link

nzakas commented Aug 11, 2015

Can you explain what each of those four stages do? It would be helpful for you to walk through an example, like:

import { foo } from "bar";

What would the process be to implement this?

@domenic
Copy link
Member

domenic commented Aug 11, 2015

This should probably use symbols, if we ever want to extend the pipeline in the future. Otherwise we may find authors squatting on names that are needed.

So loader[System.fetch] = ... maybe.

@caridy
Copy link
Contributor Author

caridy commented Aug 11, 2015

@domenic I like the idea of using symbols, but using them from System might be a problem since we will use them in Reflect.Loader class definition.

@nzakas let me try to use a real example:

<script>
// initialization of the app by executing `main.js`
System.loader.import('main.js');
</script>

As part of the internal process, loader will:

  1. first, analyze main.js and the context in which it was imported (the referrer), in this case the referrer is the page itself (top level script using imperative API), as a result, main should be relative to base of the page (which is computed by browsers based on the <base> header or the current url). As a result, main will be expanded to https://mydomain.com/main.js (assuming the page was https://mydomain.com/) by default. Hooking into this resolution process might help users to define rules to avoid the need for .js in the module name, or resolve the URL to main differently.
  2. second, once the URL for main.js is ready (from the previous step), the default fetch mechanism will be to issue a request (using fetch most likely), and wait for the result to arrive. Hooking into this fetching process might help users to define rules to fetch the actual code from different places, maybe from a local database, or using a worker, or any other alternative way of provision assets.
  3. third, once the source code is ready (from the previous step), the default translate mechanism will do nothing by default. Hooking into translate process might help users to define new rules to transpile/transform the source code before executing it.
  4. four, after getting the source code ready to be executed, the default execute mechanism will parse it, and execute it, producing a new Module exotic object. Hooking into execute process might help users to define a new rule to execute the source code, maybe setting up some variables before and after the code gets executed, maybe just executing the code in a different thread, or whatever crazy idea people will come up with.

In my mind, people will do a lot of AOP here to support various variations of modules and other assets that modules depends on, ideally that composition process for each hook is nice and easy.

@rauschma
Copy link

Why be fancy? Wouldn’t the simplest solution be subclassing plus overriding methods? Your protocol looks like there will only ever be four methods, which is why you don’t need an approach that is very flexible w.r.t. names.

To avoid name clashes, you can give hook methods the name prefix hook: hookResolve, hookFetch, hookTranslate, hookInstantiate. I expect that only the loader spec will introduce new hooks and that the only challenge is to keep hook methods and loader methods from clashing(?)

However, loader base class methods and loader derived class methods may still clash, which is why non-public methods should use symbols as keys.

@caridy
Copy link
Contributor Author

caridy commented Aug 13, 2015

@rauschma right. the problem with that approach is the default loader instance implemented by browsers, which is available thru System.loader on every realm. In theory, most users will use that instance, and will have no need to create a new instance of it, instead they can just set up some new hooks into System.loader.

@matthewp
Copy link

In theory, most users will use that instance, and will have no need to create a new instance of it, instead they can just set up some new hooks into System.loader.

The default instance should have a constructor as well. So you can extend it with:

class SuperLoader extends System.loader.constructor {
  fetch() {
    ...
  }
}

System.loader = new SuperLoader();

So Reflect.Loader is a bare class with just noops, System.loader.constructor defines the default loader behavior, System.loader is an instance of that.

@caridy
Copy link
Contributor Author

caridy commented Aug 13, 2015

System.loader = new SuperLoader();

That last line is not clear yet @matthewp, essentially because we don't know yet how that affects the behavior of <script type="module"></script>. For now, we might go with the more restrictive approach where System.loader reference can't be changed.

@matthewp
Copy link

Fair enough, so use wrapping if you want to override the default instance. You could still define the hooks in a class, create an instance and then wrap System.loader.

I personally would just tell my users to call myCustomLoader.import()... in terms of having portability for libraries that expect to be able to use System.loader, once we have the ability to do import loader from this.metadata or whatever the becomes it will mitigate the need to worry about the system loader too much.

One problem with the wrapping approach is that you really need a way to unwrap. I've run into this from time to time, you apply a hook (for testing purposes for example) and later want to remove the hook... it's much easier to simple create a new class and instantiate it for that purpose.

@caridy
Copy link
Contributor Author

caridy commented Aug 21, 2015

Ok, decision was to use symbols (thanks @domenic for the suggestion), two main reasons:

  • symbols for the hooks will signal that this is a very low level api.
  • with symbols there is not need to access the hooks via a getter when subclassing to access the internal slot.

We will stick to the same four hooks:
image

The basic usage will be:

var fetchHook = System.loader[Reflect.Loader.fetch];
System.loader[Reflect.Loader.fetch] = function (key) {
    // do something here...
};

More about this change here: #65

@caridy caridy closed this as completed Aug 21, 2015
@rauschma
Copy link

Playing devil’s advocate one last time:

  • I like the simplicity of of just modifying properties. It makes use of JavaScript’s flexibility and avoids the weight of some kind of registry pattern. There is only one caveat: two levels are conflated now – the level of the loader and the level of the hooks (with a registry, those levels are separate). I don’t think that will cause problems in practice, though.
  • On the other hand, if you use symbols you are keeping things flexible, possibly too flexible: Will there ever be more than 4 hooks? Will name clashes (= the use case for symbols) between hooks ever be an issue? Why not four properties where a name prefix prevents clashes with non-hooks: hookResolve, hookFetch, hookTranslate, hookInstantiate. That would be the simplest possible solution.

It’ll be interesting to see whether people chain hook functions (for the same hook). But doing so manually still seems better than supporting chaining via the API, e.g. via a special registry mechanism.

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

5 participants