RobLoach / component-installer

Install Web Components through Composer
https://asset-packagist.org/
Other
280 stars 35 forks source link

Put config before require.js #57

Closed bjorn-ali-goransson closed 9 years ago

bjorn-ali-goransson commented 10 years ago

As require.js API says: "define the config object as the global variable require before require.js is loaded"

As it is now, it looks for data-main before config has been applied, and assumes (!cfg.baseUrl) erroneously (see require.js:~1910 for that) even though it's been defined in config, which hasn't been applied yet because it is concatenated like so.

RobLoach commented 10 years ago

Does it conflict when you run it? There's a few variables that arn't quite yet defined in config.

bjorn-ali-goransson commented 10 years ago

I didn't quite get what you mean...

As it is now, you can't have your data-main outside baseUrl. But if you config RequireJS before including RequireJS, it works, because of that if(!cfg.baseUrl){...} part in the require.js code.

bjorn-ali-goransson commented 10 years ago

Here's the code from require.js:

//Set final baseUrl if there is not already an explicit one.
if (!cfg.baseUrl) {
  //Pull off the directory of data-main for use as the
  //baseUrl.
  src = dataMain.split('/');
  mainScript = src.pop();
  subPath = src.length ? src.join('/')  + '/' : './';

  cfg.baseUrl = subPath;
  dataMain = mainScript;
}

So it strips off the folder part of your data-main, which is bad if that folder is not also your baseUrl (which is usually isn't when using components!)

bjorn-ali-goransson commented 10 years ago

Sorry, this would really need to be in a separate pull request... can't seem to separate them now...

RobLoach commented 10 years ago

No problem, it looks pretty good, but Travis is complaining. Know what's wrong with it?

bjorn-ali-goransson commented 10 years ago

It seems like a hiccup, can one just try to rerun the test?

RobLoach commented 9 years ago

It works before, or after. require.config(), or var require. I've opened up a new pull request for this one: https://github.com/RobLoach/component-installer/pull/88