Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce overrides for main #68

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Introduce overrides for main #68

wants to merge 1 commit into from

Conversation

clarete
Copy link

@clarete clarete commented Jun 7, 2015

Debowerify doesn't really help if the bower package doesn't declare the
right files in the main section of their bower.json file. This patch
introduces the overrides section that allows the user declaring
dependencies to override the main section of the bower.json file of the
package they depend on.

This strategy was copied from main-bower-files:

https://github.com/ck86/main-bower-files#options

Debowerify doesn't really help if the bower package doesn't declare the
right files in the `main` section of their bower.json file. This patch
introduces the `overrides` section that allows the user declaring
dependencies to override the main section of the bower.json file of the
package they depend on.

This strategy was copied from `main-bower-files`:

  https://github.com/ck86/main-bower-files#options
@mkoryak
Copy link

mkoryak commented Jun 8, 2015

+1

FYI to anyone else who decides to use this fork directly: it was forked from a buggy commit in master and will not work. I applied it to latest release which made it work

@clarete
Copy link
Author

clarete commented Jun 9, 2015

@mkoryak, I'm using the master branch successfully in a small project I'm working on. What's going on in your setup?

@mkoryak
Copy link

mkoryak commented Jun 9, 2015

this plugin ends up generating this code (which i got via debugging into browserify):

require("./bower_components/angular-bootstrap/ui-bootstrap-tpls.js");
require("./bower_components/ng-tags-input/ng-tags-input.min.js");
require("./bower_components/angular-moment/angular-moment.js");
require("./bower_components/hn-nerds-components/bower.json");
require();
require();
require();
require("./bower_components/angular-scroll/angular-scroll.js");
require("./bower_components/ngstorage/ngStorage.js");
require("./bower_components/angulartics/src/angulartics.js")
require("./bower_components/angulartics/src/angulartics-adobe.js")
require("./bower_components/angulartics/src/angulartics-chartbeat.js")
require("./bower_components/angulartics/src/angulartics-cnzz.js")
require("./bower_components/angulartics/src/angulartics-flurry.js")
require("./bower_components/angulartics/src/angulartics-ga-cordova.js")
require("./bower_components/angulartics/src/angulartics-ga.js")
require("./bower_components/angulartics/src/angulartics-gtm.js")
require("./bower_components/angulartics/src/angulartics-kissmetrics.js")
require("./bower_components/angulartics/src/angulartics-mixpanel.js")
require("./bower_components/angulartics/src/angulartics-piwik.js")
require("./bower_components/angulartics/src/angulartics-scroll.js")"./bower_components/ui-router/release/angular-ui-router.js");
require("./bower_components/angular-validation-match/dist/angular-input-match.min.js");
require("./bower_components/angular-ui-select/dist/select.js");

I think it has to do with the fact that angularitics has multiple files in main - there are commits in master having to do with supporting that

@mkoryak
Copy link

mkoryak commented Jun 9, 2015

you can try this bower json (i removed some packages from our private bower, but it shouldnt matter, unless it matters that it was a dep of one of our packages and didnt actually appear in this bower.json)

{
  "name": "foo",
  "version": "0.0.1",
  "main": [
    "bower.json"
  ],
  "ignore": [
    ".tmp",
    "node_modules",
    "test",
    ".gitignore",
    "coffeelint.json",
    "gulpfile.coffee",
    "package.json"
  ],
  "dependencies": {

    "angular-bootstrap": "~0.12.0",
    "ng-tags-input": "~2.1.1",
    "angular-moment": "~0.9.0",
    "moxie": "1.2.1",
    "angulartics": "~0.17.2",
    "angular-bootstrap-checkbox": "~0.3.1",
    "angular-i18n": "~1.3.14",
    "angular-scroll-glue": "~2.0.4",
    "angular-stripe-checkout": "~2.1.1",
    "ngstorage": "~0.3.0",
    "angular-perfect-scrollbar": "https://github.com/maksad/angular-perfect-scrollbar.git#fbff161155621288d81b6d78890f13c1ff9d88a3",
    "jquery-sticky": "~1.0.1",
    "angular-ui-select": "0.9.6",
    "hn-angular-ellipsis": "~0.1.0",
    "angular-scroll": "~0.7.0",
    "angular-validation-match": "~1.4.1"
  },
  "devDependencies": {

  },
  "resolutions": {
    "squire-rte": "96bdda13ca7c4d8a0f3d15c68a6f148c01adb505",
    "angular": "~1.3.0"
  },
  "overrides": {
    "angular-i18n": {"main":["./en-us.js"]},
    "hn-core": {
      "main": ["app/index.coffee"]
    }
  }
}

@mkoryak
Copy link

mkoryak commented Jun 9, 2015

one last thing - you might consider adding some code like this to your PR:

getChildOverrides = function(bowerPath) {
    var configs, overrides;
    configs = glob.sync(bowerPath + "/**/bower.json");
    overrides = {};
    configs.forEach(function(cpath) {
      return _.extend(overrides, require(cpath).overrides || {});
    });
    _.extend(overrides, require(path.join(__dirname, "bower.json")).overrides || {});
    return overrides;
  };

because the dependencies can also have overrides declared in their bower.json files, and those need to be respected, otherwise you have to copy them over which sucks

@lattwood
Copy link

ping @eugeneware ?

@eugeneware
Copy link
Owner

I agree with @mkoryak - we probably need to make this recursive. But other than that LGTM.

@lattwood
Copy link

I'll see about adding that in my fork

@lattwood lattwood mentioned this pull request Nov 3, 2015
@pitpit
Copy link

pitpit commented Feb 23, 2016

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants