Make bower crawler more robust

[KidkArolis]

This starts implementing #39, however I wasn't sure what to do about main when bower.json is missing. That could still be read from package.json.

[unscriptable]

Argh. main. I forgot about that. There's no default value.

Perhaps we should still fetch package.json, using the browser property rules just as we do with npm, default to a moduleType of "node", and ignore dependencies?

[KidkArolis]

Updated to fetch package.json, default the moduleType and ignore deps. I think it should work ok. Bower is tricky.

I guess it could be nice to be able to override config for bower packages in the main package.json/bower.json of the project, since with bower one often needs to tweak main.

[unscriptable]

very nice. if you can put a comment in here, i'll merge it. something that explains why we don't want to pull in deps?

I guess it could be nice to be able to override config for bower packages in the main package.json/bower.json of the project, since with bower one often needs to tweak main.

We have a solution for this, actually. It's probably not documented. in the top-level bower.json/package.json, you can do this:

"rave": {
    "overrides": {
        "foo": {
            "main": "./some/other/main.js"
        }
    }
}

[KidkArolis]

Added a comment, but I wasn't very sure about why dependencies are ignored. If a package has dependencies, isn't it the case that it most likely won't work without them?

[unscriptable]

I think we're close! Can we default "main" to pkgName? That seems to be the most popular thing.

[unscriptable]

Cool. Looking good!

[unscriptable]

What's left to do on this?

[KidkArolis]

Well, I think it's done for now. However, I wanted to give this a test and I can't get the bower version of rave to work, all the modules it loads in are empty objects. Here is a gist displaying the setup (it's just the basic setup from the bower quick start guide): https://gist.github.com/KidkArolis/f02b859842ac90fd6320

[unscriptable]

Should that package.json be renamed to bower.json?

[KidkArolis]

Had it wrong in the gist (not on my laptop)

[unscriptable]

Looking into why we're seeing empty Module objects...

[unscriptable]

Ok, here's the problem: rave's default moduleType for bower packages is "globals". None of those packages pass our heuristics to set moduleType to "amd". :(

[unscriptable]

Clearly, we need to find a way to improve these heuristics or find another way to decide whether a package is "globals" or "amd" (or "node"?). Shall we open a new issue for this or continue on this PR?

[unscriptable]

...but then again, there's nothing in moment's bower.json particularly telling. Perhaps we just need to launch a new campaign urging folks to add moduleType to bower.json?

[KidkArolis]

What about fetching the main module and detecting the module type from the code?

[unscriptable]

What about fetching the main module and detecting the module type from the code?

I tried that line of thinking. A problem arises with dependencies: for global packages (those with "moduleType": "globals"), the dependencies have to be loaded before any of the global package's code can be run. Therefore, we need to know whether the package is global or whether it supports a module system (AMD/node) very early.

Hm... perhaps if we treat all bower packages as global until we execute the code? I wonder if there's a reasonable way to work that out.

[unscriptable]

perhaps if we treat all bower packages as global until we execute the code

If we treat a package with an ambiguous moduleType as globals, we will potentially load its dependencies "prematurely". The downside of loading modular package dependencies prematurely, is that we load the dependencies' main modules when they might not be actually used in code. We may also have to evaluate the code twice. Hopefully, we can avoid this.

I guess the important thing is to warn the user that there's a config problem.

Note: with global packages, the deps are declared in bower.json. Also: the main module is the package.

[unscriptable]

On another channel, @briancavalier also suggested we parse the code for moduleType signatures. This is hairy business, especially for node/CJS. This manner of code isn't uncommon at all.

However, AMD authors have learned that they must format their define()s carefully in order to allow build tools to work. Could we perhaps just sniff for AMD? Or perhaps we still sniff for both even though we know we may not always detect node/CJS?

It looks like we could sniff for moduleType somewhere in here: https://github.com/RaveJS/rave/blob/master/lib/hooksFromMetadata.js#L52-L64

[unscriptable]

@KidkArolis: you can add the following to bower.json to fix the missing moduleTypes:

  "rave": {
    "overrides": {
      "json3": { "moduleType": [ "node" ] },
      "jquery": { "moduleType": [ "amd" ] },
      "underscore": { "moduleType": [ "node" ] },
      "modernizr": { "moduleType": [ "globals" ] },
      "moment": { "moduleType": [ "node" ] },
      "fastclick": { "moduleType": [ "node" ] }
    }
  }

I chose "node" on some of those that could be either AMD or node. Just because. :)

[unscriptable]

Note: I updated my previous comment and the code snippet.

[unscriptable]

:boom: What if we ship that overrides list -- or an expanded one -- in the rave starters' bower.json files????

We could also document it in the quick start guides. Thoughts?

[unscriptable]

Still could use a few tests, but this module has been missing tests since day 1. Thanks @KidkArolis!