yeoman / bower-requirejs

[DEPRECATED] Automagically wire-up installed Bower components into your RequireJS config
375 stars 68 forks source link

Wire correctly components that has more than one main files #16

Closed cesarenaldi closed 11 years ago

cesarenaldi commented 11 years ago

Let's assume that my project depends on a component that declare more than one main file, like the following:

{
  "name": "componentName",
  "version": "1.0.0",
  "main": [
    "path/to/style.css",
    "path/to/main.js",
    "path/to/template.jade"
  ],
  "dependencies": {
    ...
  },
  "devDependencies": {
    ...
  }
}

The wired mainConfig file will end up to have those files as paths fallbacks like the following:

require.config({
  paths: {
    "componentName": [
      "components/component-name/path/to/style.css",
      "components/component-name/path/to/main.js",
      "components/component-name/path/to/template.jade"
    ]
  }
})

This is a requirejs misconfiguration. A solution can take care of each main file and create a specific path configuration (maybe using the filename as discriminating factor). See the following example:

require.config({
  paths: {
    "component-name-style": "components/component-name/path/to/style.css",
    "component-name-main":  "components/component-name/path/to/main.js",
    "component-name-template": "components/component-name/path/to/template.jade"
  }
})

In case of name matching (e.g. main.coffe and main.js) it should also include the file extension in the path key. I'll be happy to work on a pull request for this. Can you please give me some feedbacks?

sindresorhus commented 11 years ago

Hmm, hadn't considered this scenario. Your proposal does make sense, though I'm not an avid RequireJS user, so lets hear what the others think. I do think the .js file should always be "component-name" and not "component-name-main".

@passy @kevva @mlatief @iammerrick @satazor @alejandrogarciaiglesias any thoughts?

satazor commented 11 years ago

@sindresorhus in the last IRC meeting we talked about making a separate session to discuss the main property. The primary goal of the discussion will be to find a solution that is suitable for cjs, amd, multiple components, etc. I will bring this issue up in the discussion.

mlatief commented 11 years ago

I'm wondering if the current behavior is already using paths fallbacks for multiple main files, I didn't try such scenario before

In addition to the reported issue, I see that "baseUrl" won't work for multiple main files. Also, removing .js extension isn't working with multiple main files.

alejandroiglesias commented 11 years ago

@sindresorhus thanks for joining me to discussion. Not sure about which route to take. As NPM's main in package.json means the main entry point for the package, which should be one script only, and Bower's bower.json (ex component.json) is like a port of package.json, but there main means

main [string|array]: The primary endpoints of your package.

So... Thinking loud: Does it makes sense in bower.json to declare all package files/assets? Seems so. Then, should we try to find a workaround with RequireJS dev (@jrburke) to allow interoperativity? Or try to agree with Bower devs to redefine the meaning of main in bower.json and add a separate assets property into it? I'm also thinking... In a AMD world, shouldn't the main entry point (mycomponent.js) require it's assets? Following into this line of thinking, the problem seems to be fragmentation in methodologies.

robdodson commented 11 years ago

Just a heads up, it won't generate this array if you use a baseUrl and have more than one value in your main array. When it tries to create the relative file path using an array it gets a blank string.

// bower-require.js
if (config.baseUrl) {
  _.forOwn(data, function (val, key, obj) {
    obj[key] = path.relative(config.baseUrl, val); // val is an array so no relative path is generated
  });
}

In config.js you'll end up with:

// config.js
"baseUrl": "./",
"paths": {
  "componentName": ""
}

At the moment I'm working with several components with main arrays containing a js and css file. I could edit the task to only look for the js files but I guess that would exclude the folks trying to load templates and text with requirejs? Perhaps the suggestion of turning main into an object would help with that.

mlatief commented 11 years ago

@robdodson working on a fix baseUrl issue when having more than one value in main array. I'll use _.isArray() to check if it is an array, and if array iterate on the values and create relative path for each.

I'd appreciate if you have a preferable bower module with multiple main files to be used for testing. Currently I'm using anima.

robdodson commented 11 years ago

@mlatief I'm not sure how useful it is to have all of those extra paths. I don't want requirejs trying to create a path for css or images. The primary thing I want is for it to extract my main js file. Rather than iterate over the items to give them all relative paths I just did an _.find for the first .js file and removed everything else.

https://github.com/robdodson/grunt-bower-requirejs/commit/2f7ae4f74f7fc56dc0e224028b806254a828c200

mlatief commented 11 years ago

@robdodson partly agree with you, however without baseUrl the current implementation will put the array of scripts, so I'd favor being consistent with it, regardless baseUrl specified or not, until the best approach to handle the main property defined.

Also, for time being I'd suggest printing a WARN for each module with multiple files encountered, declaring that some manual post-processing might be required for it.

mlatief commented 11 years ago

@alejandrogarciaiglesias +1 to agree with Bower devs to redefine the meaning of main and add a separate assets property into it. Also, +1 to the thought about AMD, that main entry point which itself should require the assets, however I still see that some assets might not be required by the main entry point but yet should be included in the bower package for other uses, especially assets such as css/images.

robdodson commented 11 years ago

At the moment this is the best I can come up with until the main attribute is fleshed out more.

https://github.com/robdodson/grunt-bower-requirejs/commit/2497b94153f1f2fb8e03dfe0d708151007052f86

If it finds an array of js files it will create a path for each one.

main: [
  highcharts.src.js,
  highstock.src.js,
  highcharts-more.src.js
];

would produce

'highcharts': '../path/to/components/highcharts.src',
'highstock': '../path/to/components/highstock.src',
'highcharts-more': '../path/to/components/highcharts-more.src'

if main is a string, or if it only has one value it will just use obj[key] and set it equal to the value.

obviously the big downside is if you've got an array with main.js and foo.js you'll end up with a main path and a foo path. :(

sindresorhus commented 11 years ago

@robdodson this is the best option I've heard yet. Unless someone comes up with a better suggestion, let's go with this. Would you be willing to do a PR (with tests)?

robdodson commented 11 years ago

@sindresorhus will do

robdodson commented 11 years ago

PR is here https://github.com/yeoman/grunt-bower-requirejs/pull/21