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

Less plugin should share context with other loaded .less files #1

Open
zkat opened this issue Mar 4, 2015 · 4 comments
Open

Less plugin should share context with other loaded .less files #1

zkat opened this issue Mar 4, 2015 · 4 comments

Comments

@zkat
Copy link

zkat commented Mar 4, 2015

When loading files with the less plugin, it should make sure that compilation of those files groups all the .less files into a single compilation unit. The main reason for this is so already-@imported .less files get deduped and shared between all the less files that use them. There's a few details that go into this:

  • @import should be overridden (less has an API for overriding how URIs get resolved) so it resolves according to Steal pathing rules for importing modules.
  • Compilation will need to be delayed until the dependency tree is collected, and the final <style> tag should be injected into the page -before- any JavaScript executes (when using the client-side loader).
  • steal-tools should support spitting out a separate .css asset that includes the same as you'll see in the client.

As far as I've spoken with Justin, it's not clear SystemJS is able to delay execution like this right now, so we might have to do some patching on that as well. For reference, @shcarrico did this previously (or things like it) for both old-steal and for webpack. This has been tricky to implement correctly in both of those systems, so I don't really expect it to be a simple/easy task for new steal either, but it's a fairly important feature, imo. The need for it will become even more obvious once we have a bunch of standalone can.Components getting distributed.

See stealjs/steal#346 in reference to this.

@matthewp
Copy link
Member

matthewp commented Mar 4, 2015

What is the purpose of having it as a single compilation unit?

I agree that we should override less's own import mechanism to prevent duplicate loading (not sure how that would work).

But single compilation would be very difficult if not impossible. The reason is you don't know when the loader is really finished. So you can't wait for all less files to be ready.

If your concern is duplication that should be fixed by the minifier anyways. In dev it really shouldn't matter. I think it would be interesting if possible but I can't think (off the top of my head) of how to do that.

@matthewp
Copy link
Member

matthewp commented Mar 5, 2015

Thinking a bit more about this it might be possible. How I would approach this is to create a separate loader that loads but doesn't execute modules to use as a trace. Collect all of the less/css. Once it's done you can copy over the modules into the main loader and reset their state to translate. From there I think loading would pick up at the current state. Here's some pseudocode (definitely not working, but a start):

function css(loader){
  var import = loader.import;
  loader.import = function(name){
    var cssLoader = loader.clone();
    cssLoader.csses = [];

    // Prevent execution.
    var instantiate = cssLoader.instantiate;
    cssLoader.instantiate = function(){
      if(/\.css/.test(load.name)) {
        cssLoader.csses.push(load.source);
      }

      // Prevent modules from being executed.
      return Promise.resolve(instantiate.call(this, load)).then(function(result){
        result.execute = function(){
          return cssLoader.newModule({});
        };
        return result;
      });
    };

    return cssLoader.import(name).then(function(){
      // An array of css contents. Insert them into the page somehow.
      var csses = cssLoader.csses;

      // Copy over the modules. and set the state to translate
      var module;
      for(var i = 0; i < cssLoader._loader.modules.length; i++) {
        module = cssLoader._loader.modules[i];
        module.state = 'translate';
        loader._loader.modules.push(module);
      }

      // Now run load to cause execution of the copied over modules.
      return loader.load(name);
    });
  };
}

if(typeof System !== "undefined") {
  css(System);
}

@wclr
Copy link

wclr commented Mar 7, 2015

You could create less plugin that uses another FileManager that uses file cache shared across the process.

@justinbmeyer
Copy link
Contributor

What does this mean and why should we do this?

Sent from my iPhone

On Mar 7, 2015, at 5:02 PM, Alex [email protected] wrote:

You could create less plugin that uses another FileManager that uses file cache shared across the process.


Reply to this email directly or view it on GitHub.

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

No branches or pull requests

4 participants