Closed afram closed 8 years ago
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:
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.
@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 https://github.com/webpack/webpack/issues/883 to be handled..
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"
@afram If you can find something that makes it work right now - I'll insta-merge it and publish a fix to npm
@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.
@afram - Merged. Let me know if you need anything else.
@eiriklv cheers just tested again and it works.
Will look into migrating over to NPM deps.
@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.
oh nice - cheers :-)
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.
@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.
@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?
@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 :-)
fixed in 3.0.0
Wondering why this library and most (all?) the libraries it depends on are pulling in Github dependencies?
They currently don't support semver versioning, and this leads to unpredictable results.
For instance this change from 2 days ago in a downstream dependency broke react-masonry-component for us. We had to shrinkwrap to enforce version numbers from within git dependencies.
https://github.com/desandro/matches-selector/commit/7582db4413229cdbe36bfe2b6228b210296b6d3d
What are the thoughts on moving dependencies over to npm?