outlayer icon indicating copy to clipboard operation
outlayer copied to clipboard

Wiredep bug - item.js missing in bower.json main

Open Lucas-C opened this issue 10 years ago • 16 comments

Hello,

It appears that commit 914f1f2 introduced a bug: https://github.com/metafizzy/outlayer/commit/914f1f2fa9874935dc7723a5a1171e0f78a689e7#diff-a63318b17de3bf77b5cd6eb71e7aae44R46

How can window.Outlayer.Item be defined before assigning the result of the factory call to window.Outlayer ?

My Karma tests fail on this when launching PhantomJS, with the following error:

Running "karma:dev" (karma) task
INFO [karma]: Karma v0.12.31 server started at http://localhost:9876/
INFO [launcher]: Starting browser PhantomJS
INFO [PhantomJS 1.9.8 (Windows 8)]: Connected on socket _xqtqhxXOy8qR62SOmNJ with id 26908454
PhantomJS 1.9.8 (Windows 8) ERROR
  TypeError: 'undefined' is not an object (evaluating 'window.Outlayer.Item')
  at src/main/bower_components/outlayer/outlayer.js:42

Or did I got the root cause wrong ?

Lucas-C avatar Apr 16 '15 08:04 Lucas-C

Actually this already happened in v1.3.0

Looks like @dimerman had a similar issue back in 2013 : https://github.com/metafizzy/outlayer/commit/a0d5481f1ea0bac982f2d22e90118ac40fd3e071

Seems like I "need to have item.js before outlayer.js".

But outlayer is just a dependency of packery for me : shouldn't item.js be automagically included as a dependency by bower ? I does not appear when I run bower list.

Lucas-C avatar Apr 16 '15 08:04 Lucas-C

bower's main has been poorly defined in the past. While previous versions of Outlayer did include item.js in main, Outlayer v1.3 has adopted a new specification for main. See bower/bower.json-spec#43.

Unfortunately, this may break build tools. Thanks for reporting this issue.

desandro avatar Apr 16 '15 18:04 desandro

I was affected by this as I use wiredep for loading the bower dependencies into my web projects. I can understand the single file entry point philosophy in bower only if the file actually serves as a single entry point.

Either a single concatenated distribution file or a single file which requires all others in some way.

Until this is resolved I have added the below override to my growing list of overrides to patch scenarios like this one.

Hopefully this helps others.

  "overrides": {
    "outlayer": {
      "main": [
        "item.js",
        "outlayer.js"
      ]
    }
  }

EHLOVader avatar Apr 20 '15 01:04 EHLOVader

Thanks @EHLOVader ! That did the trick for me

Lucas-C avatar Apr 21 '15 15:04 Lucas-C

Sorry to bother @EHLOVader but i'm a noob. Does the override snipet you showed goes on the bower.json after the dependencies or somewhere different?

jesusjackson avatar Aug 03 '15 09:08 jesusjackson

@jesusjackson you've got it!

You just add an overrides property to your bower.json object. It can go anywhere afaik. I think the property is "unofficial" but I can't see it going anywhere any time soon. So many people depend on it for things like this. Here are details from Roots.io, was the best doc I found on the topic.

https://roots.io/using-bootstrap-with-bower-how-to-override-bower-packages/

EHLOVader avatar Aug 03 '15 15:08 EHLOVader

Thanks for the quick answer @EHLOVader, but apparently grunt is ignoring the overrides

"overrides": {
    "isotope": {
      "main": ["./dist/isotope.pkgd.min.js"]
    },
    "outlayer": {
      "main": [
        "item.js",
        "outlayer.js"
      ]
    }
  }

Is generating these two lines instead of the ones i specified on the override:

    <script src="bower_components/outlayer/outlayer.js"></script>
    <script src="bower_components/masonry/masonry.js"></script>
    <script src="bower_components/isotope/js/isotope.js"></script>

jesusjackson avatar Aug 04 '15 05:08 jesusjackson

@jesusjackson That is certainly odd. Perhaps not using a ./ relative location for your main file.

This override that I have here is only a fraction of the full override I have started using myself. Since the definition of the main property was changed and only single entry points were recommended I have had to override many projects which weren't built for a single entry point, like this one.

I think a pretty complete override that worked for me is here https://github.com/metafizzy/isotope/issues/879#issuecomment-94335043

I had to include the dependencies on that one for jquery-bridget support.

If that doesn't work what are you using for grunt to handle the dependencies? I use gulp myself, but I see gulp-bower-install mentioned in a few comment threads that suggest switching to gulp-wiredep or just newer versions.

Good luck.

EHLOVader avatar Aug 04 '15 19:08 EHLOVader

No use @EHLOVader, apparently bower hates me. I use wiredep, but even with your full override. I'll put my bower.json here. Thanks for all the help.

"overrides": {
    "isotope": {
      "main": [
        "js/item.js",
        "js/layout-mode.js",
        "js/isotope.js",
        "js/layout-modes/vertical.js",
        "js/layout-modes/fit-rows.js",
        "js/layout-modes/masonry.js"
      ],
      "dependencies": {
        "get-size": ">=1.1.8 <1.3",
        "matches-selector": ">=1 <2",
        "outlayer": "1.3.x",
        "masonry": "3.2.x",
        "jquery": ">=1.4.3 <2",
        "jquery-bridget": "1.1.x"
      }
    },
    "outlayer": {
      "main": [
        "item.js",
        "outlayer.js"
      ]
    }
  }

jesusjackson avatar Aug 05 '15 12:08 jesusjackson

Thanks @jesusjackson this helped me out.

danimalweb avatar Oct 26 '15 21:10 danimalweb

For anyone using grunt-bower-concat try something like this.

module.exports = function(grunt) {
    grunt.initConfig({
        bower_concat: {
            all: {
                dest: './_bower.js',
                cssDest: './_bower.css',
                exclude: [
                ],
                dependencies: {
                },
                mainFiles: {
                    'isotope': [
                        "dist/isotope.pkgd.min.js"
                    ],
                    "outlayer": [
                        "item.js",
                        "outlayer.js"
                    ]
                },
                bowerOptions: {
                    relative: false
                }
            }
        }
    });

    grunt.loadNpmTasks('grunt-bower-concat');

    grunt.registerTask('default', ['bower_concat']);
};

OmgImAlexis avatar Nov 17 '15 01:11 OmgImAlexis

@OmgImAlexis You shouldn't do it this way, you will end up including Outlayer twice in your generated file, because isotope.pkgd.min.js already includes it.

Here's my version:

module.exports = function(grunt) {
    grunt.initConfig({
        bower_concat: {
            yourBuildTarget: {
                mainFiles: {
                    // outlayer (and isotope) have a main-definition that conflicts with non-module loader setups.
                    // See https://github.com/metafizzy/outlayer/issues/33
                    'outlayer':   ['item.js', 'outlayer.js'],
                    'isotope':    [
                        'js/item.js',
                        'js/layout-mode.js',
                        'js/layout-modes/fit-rows.js', 'js/layout-modes/masonry.js', 'js/layout-modes/vertical.js',
                        'js/isotope.js',
                    ],
                }
            }
        }
    });
    // …
}

PhilippSoehnlein avatar Feb 01 '16 10:02 PhilippSoehnlein

Isn't there an official resolution for this yet?

lucasengel avatar Feb 07 '16 00:02 lucasengel

It basically is resolved.

From the comment above: https://github.com/metafizzy/outlayer/issues/33#issuecomment-93809453

bower's main has been poorly defined in the past. While previous versions of Outlayer did include item.js in main, Outlayer v1.3 has adopted a new specification for main. See bower/spec#43.

Unfortunately, this may break build tools. Thanks for reporting this issue.

There is still some discussion over here https://github.com/bower/spec/issues/47 on ways to improve this beyond just using overrides, but overrides or using the pkgd dist file is the official solution afaik.

EHLOVader avatar Feb 07 '16 23:02 EHLOVader

Thanks @EHLOVader!

For anyone using Grunt, here's my version on fixing Masonry dependencies:

wiredep: {
  options: {
    "overrides": {
      "ev-emitter": {
        "dependencies": {
          "jquery-bridget": ">=2.0.0"
        }
      },
      "outlayer": {
        "main": [
          "item.js",
          "outlayer.js"
        ]
      }
    }
  }
}

lucasengel avatar Feb 09 '16 13:02 lucasengel

Thanks @EHLOVader it worked for me.

lgrassini avatar Feb 11 '16 22:02 lgrassini