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

Instantiate result form #9

Closed
guybedford opened this issue Jan 20, 2015 · 15 comments
Closed

Instantiate result form #9

guybedford opened this issue Jan 20, 2015 · 15 comments

Comments

@guybedford
Copy link

Following http://whatwg.github.io/loader/#instantiate, it seems like the instantiate result is just a single factory function.

Surely that would mean that dependency analysis can't be done to perform the zebra-striping?

Would help a lot to know about this.

@dherman
Copy link

dherman commented Feb 6, 2015

Sorry, been kind of heads down. So I realized that zebra striping can be simplified quite a bit by pushing the smarts out of the loader semantics and into userland. In particular:

  • Dynamic modules do not have dependencies understood by the system. Instead a dynamic module that depends on other modules can use the dynamic API's for requiring them.
  • As you say, the factory function does not specify dependencies.
  • As with the zebra striping, this precludes cycles between "stripes" but without having to do a big hairy graph algorithm. Instead, dynamic modules are just leaves in the dependency graph that the system understands.

Does this make sense? I'm happy to try to clarify further.

@caridy
Copy link
Contributor

caridy commented Feb 6, 2015

I know that @guybedford has managed to get the zebra striping working on SystemJS, but I agree with @dherman that this is probably overkill, especially now that the transpilation processes are becoming more and more advanced. For executable code, you will probably transpile to a single format that works well with the loader that you use rather than mixing the formats in the runtime.

The other use-case that is probably more interesting is to use the loader for non-executable code, to reuse the loader pipeline to accommodate such things, without needing to bring another loading mechanism into the page. But can those non-executable modules specify dependencies? I will argue that they don't, that they can be folded/compiled by a build process (read css, templates, etc) or they can be used thru a specialized loader.

@matthewp
Copy link

matthewp commented Feb 6, 2015

Will there be another way to get a module's dependencies? Currently the way we're doing builds is to override instantiate preventing execution and build a dependency graph based on the deps from instantiate. How would you do this if instantiate just returns a factory function?

Also, I don't think transpilation should be a requirement to develop for the web. Unless it can easily be done in the browser (and I don't think it can) we should leave transpilation as an optimization for when you are prepared to deploy.

@dherman
Copy link

dherman commented Feb 7, 2015

@matthewp Can you link me to some lines of code demonstrating the instantiate override that prevents execution to do builds? I want to understand better.

@matthewp
Copy link

matthewp commented Feb 7, 2015

Sure, here is an example. But some pseudo code might be more helpful. Essentially what I want to be able to do is:

var graph = {};

var instantiate = loader.instantiate;
loader.instantiate = function(load) {
  return Promise.resolve(instantiate.call(this, load)).then(function(instantiateResult) {
    graph[load.name] = {
      load: load,
      deps: instantiateResult.deps
    };

    return {
      deps: instantiateResult.deps,
      execute: function() { // Empty execute
        return new loader.newModule({});
      }
    };
  });
};

This doesn't work for es6, have to parse the source. Could do that for dynamic modules as well but would rather not have to.

I understand that this change is intended to simplify things, is there another way of achieving what I need without the old instantiate?

@matthewp
Copy link

matthewp commented Feb 7, 2015

To summarize, what I'm really looking for (and what was achievable in the old instantiate) is:

  1. A way to prevent execution of modules. Which with this new instantiate would be as simple as returning a noop. Would this work for es6 as well?
  2. A way to get the loader dependency graph. If there were another API to get a module's dependencies that would work.

@guybedford
Copy link
Author

To clarify on the instantiate behaviour - can you confirm this means that there is no way a dynamic module can be dependent on an ES6 module without using a dynamic import like System.import?

The main implication of this seems to be that it would completely stop users from continuing to write CommonJS in ES6 environments as they won't be able to load ES6 dependencies.

@johnjbarton
Copy link

it would completely stop users from continuing to write CommonJS in ES6 environments as they won't be able to load ES6 dependencies.

@guybedford in your first sentence you say there is a way, System.import, and in the second sentence you say there is no way. I wonder if you meant something else or I misunderstood you.

@guybedford
Copy link
Author

I mean that it would stop them loading ES6 modules statically, which would be enough of a reason not to write CommonJS as you typically don't want to provide all APIs as promises that do a System.import first.

@dherman
Copy link

dherman commented Feb 15, 2015

@guybedford No, it means that that functionality can be provided automatically by an interop framework like SystemJS by taking the stated declarative dependencies and automatically invoking .import to get them. As opposed to the functionality being provided for SystemJS by the loader API. This is just a shifting of responsibilities but to the user program there is no loss of functionality.

@guybedford
Copy link
Author

@dherman I see how this implicitly handles zebra striping. But in the case of mixed cycles, we would effectively just get a loop then? The nice thing before was we could easily throw in this case - https://github.com/ModuleLoader/es6-module-loader/blob/master/src/loader.js#L603.

The more I think about it though the more I'm not sure having CommonJS loading ES6 is a good idea actually at all, and just having users write ES6 to load ES6. What are your thoughts on leaving this feature out entirely?

@dherman
Copy link

dherman commented Feb 17, 2015

@guybedford I think going in both directions is important for adoption. If people can't write using ES6 without cutting off their clients to only ES6 it creates a deterrent against using ES6. But I think the cycle case is so rare as to be no big deal in practice. The loop is certainly not ideal, but couldn't you implement the cycle detection in the tooling and throw? The dependency graph is declared statically so you should be able to detect cyclic zebra crossings.

And you have to admit, the opportunity to have a CyclicZebraCrossingError is too good to pass up...

@guybedford
Copy link
Author

Thanks so much, this all sounds great, happy to investigate static cyclic zebra crossing warnings etc!

To clarify the exact API form:

System.hook('instantiate', function(key, source, metadata) {
  return function() {
    return new Reflect.Module({ custom: 'module' });
  }
});

(with the addition of metadata assumed). Please let me know if that seems correct.

@dherman
Copy link

dherman commented Mar 11, 2015

Yep, looks right!

@dherman dherman closed this as completed Mar 11, 2015
@matthewp
Copy link

@dherman Have you thought about my dependencies question above? Should I open a separate issue to discuss this?

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