cujojs / curl

curl.js is small, fast, extensible module loader that handles AMD, CommonJS Modules/1.1, CSS, HTML/text, and legacy scripts.
https://github.com/cujojs/curl/wiki
Other
1.89k stars 216 forks source link

Preloading poly + next()/module fails in IE8 #216

Closed webpro closed 10 years ago

webpro commented 10 years ago

In IE8, this usually gives me an error:

curl({preloads:['poly']}).next(['wire'], function(wire) {
    console.log(wire);
});

The error points to a line in wire.js that indicates indeed the poly hasn't filled the gap yet:

hasOwn = Object.prototype.hasOwnProperty.call.bind(Object.prototype.hasOwnProperty);

Although not recommended, the following should work just as well, right? Without the preloads config (also fails in IE8):

curl(['poly']).next(['wire'], function(wire) {
    console.log(wire);
});

This just works, btw:

curl(['poly'], function() {
    console.log(Function.prototype.bind);
});

just like this:

curl({preloads: ['poly']}, [], function() {
    console.log(Function.prototype.bind);
});

Things start to fail in IE8 when I'm actually trying to load a module that needs the polyfills:

curl({preloads: ['poly']}, ['wire'], function(wire) {
    console.log(wire);
});
unscriptable commented 10 years ago

Hey @webpro!

I guess I should document that this combination is not supported:

curl({preloads:['poly']}).next(['wire'], function(wire) {
    console.log(wire);
});

However, if the following is failing, then there's a bug somewhere:

curl({preloads: ['poly']}, ['wire'], function(wire) {
    console.log(wire);
});

You're saying that the above code snippet fails intermittently in IE8?

-- John

webpro commented 10 years ago

I found that indeed your snippet works fine in isolation.

So I dug a little deeper, and I'm guessing it has a lot to do with #215. This is what I did:

curl.config({ preloads: preloads});

define('namedModule', ['dep'], function() {});

As soon as the named module has an async dependency (i.e. not defined yet), it fails. So, this also works fine:

define('namedModule', [], function() {});

curl({preloads: ['poly']}, ['main'], function() {
    console.log(typeof Function.prototype.bind)
});

So my issue is clearly around "named modules". Also see this boot script for some more context.

Am I right saying that only in a future version of curl this might be possible (as per your comment in #215)?

unscriptable commented 10 years ago

Am I right saying that only in a future version of curl this might be possible (as per your comment in #215)?

Yes.

I like your boot script. I am wondering if there is a way to remove the named modules from it. There is probably a way. Without further context, I can't tell for sure how much work that might be, though.

webpro commented 10 years ago

OK, thanks! At least I know the issue. Will go and think about another way to boot. Worst case is I'll need to stick with require,js until curl allows me to do something similar.

unscriptable commented 10 years ago

Are the modules named because you need two modules in the file? From what I can see, it looks like you're declaring two modules to avoid the need to declare the core package's main module? Why not just declare a package before loading the boot file?

curl.config({ 
    packages: {
        // declare core package
        core: { location: 'foo/bar', main: 'core' }, 
    },
    // load the core package
    main: ['core']
});

Does this help?

-- J

webpro commented 10 years ago

Yeah, getting close. I'm using the global curl configuration option before loading curl itself.

Do you also have a suggestion to conditionally map one path to another? E.g. core/util points to foo/bar/util.js by default, but foo/bar/util.legacy.js if legacy environment. Something like this:

curl = {
    paths: {
        'core/util': 'core/util' + (legacy ? '.legacy' : '')
    },
    packages: {
        core: { location: 'foo/bar', main: 'core' }
    }
};

Curl doesn't have a mappings configuration or similar, right?

webpro commented 10 years ago

A workaround would be to just use util (instead of core/util), so that would be OK:

curl = {
    preloads: preloads,
    paths: {
        util: basePath + 'util' + (legacy ? '.legacy' : '')
    },
    packages: {
        core: { location: basePath, main: 'core' }
    }
};
unscriptable commented 10 years ago

What you posted should work:

curl = {
    paths: {
        'core/util': 'core/util' + (legacy ? '.legacy' : '')
    },
    packages: {
        core: { location: 'foo/bar', main: 'core' }
    }
};

curl sorts by the specificity of paths/packages. Specificity is measured by the number of slashes, so "core/util" will be used over "core".

Note that the path mapping performed will affect the entire core/util module path. Therefore, if there are modules in the core/util module path (e.g. core/util/foo or core/util/bar), they will also get mapped (e.g. to core/util.legacy/foo.js and core/util.legacy/bar.js).

webpro commented 10 years ago

Heck, you're right. Thanks! This boot script is working fine here, got things set up properly without named modules. Win!

unscriptable commented 10 years ago

woot!