disabling normalize breaks static build

[yoavniran]

continuing from the thread at: jspm/jspm#1314

In the project im working on, im doing dynamic requires, as in, the name of the module I require is constructed at runtime based on configuration that is provided at build time. I had trouble getting this to work but found an almost adequate solution by setting the compile option "encodeNames" to false. However this left a couple of problems, 1) the ".js" extension was left in the dependency names (annoying be workable) and 2) the aliases from my config.js were ignored.

My config.js looks something like this:

System.config({
   //...
     meta: {
          "jtvw.integrations/integrationsProvider":{
            "format": "cjs",
            "deps": ["jtvw.integrations/facebook/fbIntegrator",
                "jtvw.integrations/twitter/twitterIntegrator"]
        },
    },
     map: {
          //...
       "jtvw.integrations": "src/integrations",
         //...
      }
});

So in my code (integrations provider) I would like to do something like this:

require("jtvw.integrations/" + name + "/" + config.intName); //ex: name = facebook, config.intName = fbIntegrator

With just encodeNames set to false, I get in the bundled code:

$__System.registerDynamic("src/integrations/integrationsProvider.js", ["web-commons", "promise", 
"utils", "Logger", "src/integrations/facebook/fbIntegrator.js", "src/integrations/twitter/twitterIntegrator.js"], 
true, function($__require, exports, module) {
//...

The dependency names dont match my config or my code... 😞

So I realized that passing "normalize = false" will make the dependency names stay as I want them:

$__System.registerDynamic("src/integrations/integrationsProvider.js", ["web-commons", "promise", 
"utils", "Logger", "jtvw.integrations/facebook/fbIntegrator", "jtvw.integrations/twitter/twitterIntegrator"], 
true, function($__require, exports, module) {
//...

but... now I have a bigger problem, the require calls inside the module arent renamed correctly, even for the normal (not dynamic) dependencies and the resulting code is:

$__System.registerDynamic("src/integrations/integrationsProvider.js", ["web-commons", "promise",
 "utils", "Logger", "jtvw.integrations/facebook/fbIntegrator", "jtvw.integrations/twitter/twitterIntegrator"],
 true, function($__require, exports, module) {
  ;
  var global = this,
      __define = global.define;
  global.define = undefined;
  module.exports = (function() {
    "use strict";
    var _ = require("web-commons").common,
        Promise = require("promise").Promise,
        utils = require("utils"),
        logger = require("Logger").getLogger();
//...

notice that the module function receives "$__require" while inside the calls are left as "require" which of course fails with "require is not defined" Error.

Am I missing something here? does disabling normalize also means the require method isnt renamed on purpose or is it a bug?

thanks.

[guybedford]

Static (SFX) builds require that all modules are statically linked. That is, that every require in the build can be fully analysed. normalize: false is therefore not compatible with static builds.

Static builds can still do dynamic loading by calling System.import('x'). Alternatively, you can output the static build as a CommonJS module, and then it may be possible to access the top-level global require statement of the environment. It may be interesting to work out a workflow here, but I don't want to speculate unless that would be useful to you.

Let me know how the above sounds, and what route you think might be best here.

[yoavniran]

hi @guybedford
as far as I can tell with the compile props: encodeNames and normalize set to false, everything was correct in my bundle except for the require calls not being correctly converted to the global name ($__require).

My aim is to have everything bundled statically and not have additional calls to load dependencies.
Im not after dynamic loads but rather dynamically constructing the module's path at runtime only. During dev-time I dont know which of the dependencies will be configured. This is decided during build time.

My build process creates the meta configuration in the config.js (based on the configuration it receives) before compiling so the bundling does have all the correct dependencies in the final sfx bundle.

The problem with the standard (static) build is that all paths are encoded into sequential numbers so I used encodeNames = false. But this results in a bundle with paths that do not adhere to the aliases in my config file and with the ".js" extension (shown above).

So I turned to normalize = false. This worked perfectly for the dependency paths in the bundle = aliases, no ".js". But all the require calls fail (require not defined). I didnt see any other issue with static build and disabling normalize to be honest.

I think what I would have really liked is the ability to mention in the System Config that for meta dependencies I would like the compiler to leave the path as is instead of encoding+normalizing.
Something along the lines of:

System.config({
   //...
     meta: {
          "jtvw.integrations/integrationsProvider":{
            "format": "cjs",
            "deps":{
                "dynamic": true, //DONT ENCODE/NORMALIZE THE PATHS FOR THESE DEPS
                "deps":  ["jtvw.integrations/facebook/fbIntegrator",
                "jtvw.integrations/twitter/twitterIntegrator"]
               }
        },
    },
     map: {
          //...
       "jtvw.integrations": "src/integrations",
         //...
      }
});

This way it will just leave the ones I want to construct at runtime and all the rest can be optimized.

Hopefully I did a better job explaining myself this time.

Thanks!

[guybedford]

Ah right, I see what you are saying, although I'm still not sure I agree in principle!

The change in 2dc0657 should enable this. Let me know if you're able to test it out on master.

[yoavniran]

sure, will give it a try and let you know.

out of curiosity, which part dont you agree with? 😄

[yoavniran]

@guybedford
unfortunately the commit you mentioned isnt enough to get the static bundle working.

first, there's an if statement in the cjs compiler that checks if normalize is enabled and only then runs the RequireTransformer. Removing that if seems to have solved that problem.

second (bigger) problem I see is that my CSS dependencies are mangled as they are still (partly) normalized.

$__System.registerDynamic("src/init.js", ["assets/css/widget.css!", "assets/css/outer.css!", "web-commons", "Logger", "utils", "jtvw.WidgetManager", "promise", "mvc"], true, function($__require, exports, module) {
  ;
  var global = this,
      __define = global.define;
  global.define = undefined;
  $__require('assets/css/widget.css!github:JoinTV/jtv-systemjs-css-plugin@0.3.2');
  $__require('assets/css/outer.css!github:JoinTV/jtv-systemjs-css-plugin@0.3.2');
  (function() {
    "use strict";
//...

in the original code the CSS requires look like this :

require("assets/css/widget.css!");
require("assets/css/outer.css!");

notice the dependency list contains the original name (without the plugin path) and the require inside the module function has the full name. At runtime the require method fails because it doesnt find a match for the full path in its deps list...

[guybedford]

@yoavniran these are then exactly the edge cases of not running normalization, and there will be more after these.

[yoavniran]

bummer...
so going back to my enhancement request, it would be really useful for my case to specify for specific dependencies that their paths shouldnt be encoded/normalized (in system config).

Right now all paths in my bundle arent encoded(shortened) because i have just a handful that cant be encoded.

My workaround for leaving normalize on is that I've written a method similar to the one below that does the right thing at runtime but its very ugly and wish I didnt need to use it...

function dynamicRequire(path, sysRequire){
        var res;

        if ("System" in global && System.import) { //dev
            res = System.import(path);
        }
        else {//production
            path = path.replace("jtvw.", "src/"); //systemjs builder doesnt provide the alias in the bundled code so need to replace it here :(
            path += ".js"; //systemjs builder leaves the .js in the dependency names
            res = Promise.resolve(sysRequire(path)); //must use the require() from the caller
        }

        return res;
    }

[guybedford]

Playing around with custom normalization is really not going to be much fun.

I'd rather look at supporting normalizing conditional expressions like:

`require('x/' + 'y.js')`

by replacing this with:

require('normalized/path/to-x/' + 'y.js')

Would something like that be useful here?

[yoavniran]

With what you're suggesting would this be correct?

//will work:
const MyModule = require("app.integrations/" + "myIntegrator.js");

//wont work(?):
const myModulePath = "app.integrations/" + "myIntegrator.js";
const MyModule = require(myModulePath);

Also, would I be able to use the aliases in my config for the "deps" as well as in my code? As now its not an option for the dynamically constructed paths.

[guybedford]

Yes exactly, where app.integrations can be config-aliased.

[yoavniran]

Thanks @guybedford

can you confirm for me the correctness of the code sample in my previous post? would the second part not work? If so, I think this approach will be a bit problematic if its that specific that it actually relies on string concatenation alone.

[guybedford]

Yes it wouldn't work in the second case, as implementing deeper AST-level code analysis is not really a top-priority here.