Why are dependencies linked to Github instead of NPM and semver? about react-masonry-component HOT 15 CLOSED

Hi @afram - this whole module is a bit hacky as you have experienced.

The reason for the github deps is that the ones that originally were on npm (including masonry itself) didn't work with CommonJS.

To make the whole thing work I had to clone the whole dependency tree (and some forks that had implemented CommonJS/npm support) and make the necessary changes.

Also - because of mixed use of AMD/require.js (bower) and CommonJS (npm) it didn't work with Webpack (defaults to the first supported format, which when using the UMD boilerplate is AMD). If you look at the forks and the last commit(s) you'll see what i mean.

If you want to help clear things up and get a working version which uses only npm I'd be happy to merge it / make you a collaborator with access :-)

For more info check out these issues:

• eiriklv/react-masonry-mixin#1
• eiriklv/react-masonry-mixin#4

from react-masonry-component.

Yeah I see - all a bit frustrating.

I'd say there's an argument to update UMD boilerplate to put CJS first.

I'm looking into migrating this lib to npm deps.

I suspect that it should just work for Browserify users, though webpack users will need some more config.

from react-masonry-component.

@afram That's the case - it just works for out of the box for browserify, but creates a lot of issues with Webpack. So back when I was working with this I couldn't find an easy solution without rewriting the UMD templates. Still waiting for webpack/webpack#883 to be handled..

from react-masonry-component.

Another option at least for now while a better solution appears would be to reference explicit versions from github deps.

"matches-selector": "desandro/matches-selector#v1.0.3"

from react-masonry-component.

@afram If you can find something that makes it work right now - I'll insta-merge it and publish a fix to npm

from react-masonry-component.

@eiriklv I made a PR against eiriklv/outlayer which should sort this issue out. I believe No version changes are required for this to start working.

This isn't ideal, but it's probably an OK interim step to keep things working for everyone until a more robust solution is developed.

from react-masonry-component.

@afram - Merged. Let me know if you need anything else.

from react-masonry-component.

@eiriklv cheers just tested again and it works.

Will look into migrating over to NPM deps.

from react-masonry-component.

@afram Awesome :-) I haven't got much time to work on this ATM, so I've made you a collaborator (both github and npm) in case you want to push fixes.

from react-masonry-component.

oh nice - cheers :-)

from react-masonry-component.

As you've noted, the issue lies with how Webpack deals with nested dependencies in issue 883. Rather than maintaining forked version of every dependency of Masonry, a simpler solution may be to use webpack.config.js. This way you can use original dependencies, as we can raise awareness around the Webpack issue. I've added Webpack instructions to Masonry - Appendix

Install Masonry and imports-loader with npm.

npm install masonry-layout imports-loader

In your config file, webpack.config.js, use the imports loader to disable define and set window for masonry-layout modules.

// npm v2
module.exports = {
  module: {
    loaders: [
      {
        test: /masonry-layout/,
        loader: 'imports?define=>false&this=>window'
      }
    ]
  }
};

// npm v3
module.exports = {
  module: {
    loaders: [
      {
        test: /masonry|fizzy\-ui\-utils|desandro\-|outlayer|get\-size|doc\-ready|eventie|eventemitter/,
        loader: 'imports?define=>false&this=>window'
      }
    ]
  }
};

(This hack is required because of an issue with how Webpack loads dependencies.)

You can then require('masonry-layout').

// main.js
var Masonry = require('masonry-layout');

var msnry = new Masonry( '.grid', {
  // options...
});

Run webpack.

webpack main.js bundle.js

jQuery plugin functionality need to be installed separately, similar to using Browserify.

from react-masonry-component.

@desandro - Thanks for the alternative fix!

Neither of the solutions are any elegant IMO. You should be able to npm install without much fuss. A shame that the coincidence of AMD's if statement in the UMD spec preceding CommonJS causes these problems. Especially since Bower seems to be a dying beast. If it was the other way around this wouldn't be a problem.

from react-masonry-component.

@eiriklv That solution might be the way to go - It's a few lines of webpack config (which sucks) but we get a lot more benefits from semver and maintained dependencies.

Thinking it'd be good to create 1x and 2x branches in case support is needed for legacy versions, and bump this to v3 - what do you think?

from react-masonry-component.

@afram As long as it's documented in the README and we branch off - then I'm good. Go for it if you have the capacity :-)

from react-masonry-component.

fixed in 3.0.0

from react-masonry-component.