search

eiriklv / react-masonry-component

A React.js component for using @desandro's Masonry
MIT License
1.44k stars 145 forks source link

Notes on using webpack in the readme #50

Closed hburrows closed 7 years ago

hburrows commented 8 years ago

Are the Usage with webpack instructions in the README still applicable. I use webpack, I didn't install/configure the imports-loader and everything works fine. To the contrary... if I install and configure the imports-loader per the instructions webpack stops working. I'm asking in case I'm overlooking something important.

hburrows commented 8 years ago

Maybe this was necessary with older versions of masonry and imagesloaded ??

afram commented 8 years ago

Hi @hburrows

Which version of Webpack are you using?

At the time of writing the Readme, the import order Webpack used was AMD then commonjs. This meant that some dependencies didn't play nice (imagesloaded plus others)

Things may have changed though, so thanks for bringing this up :-)

hburrows commented 8 years ago

@afram Thanks for the followup. I'm currently using Webpack version 1.13.2 (which is the most recent in the 1.X line).

afram commented 8 years ago

Hmm, I wouldn't have thought they would not change such a critical implementation detail without a major version bump, so I'd be very surprised if the import order has changed in that version. I'll check my setup when I have a second and report back.

yoadsn commented 8 years ago

I also installed the npm package without any changes to my webpack configuration (I don't use the import-loader). It works for me, I cannot find any noticeable problems.

I did look into all the top level packages used by this component and all seem to support webpack officially. Probably that is what changed.

The only thing which could still behave weird is this package's conditional "require" statements which depend on the "isBrowser" boolean. I am not sure how webpack will handle that but I assume it would still require them which would mean they are eventually bundled in all the builds (server side and client side if you do both).

I am using that in SSR scenario and all seems to work as said before.

webpack: 1.13.3
-- react-masonry-component@4.3.1
  +-- element-resize-detector@1.1.9
  | `-- batch-processor@1.0.0
  +-- imagesloaded@4.1.1
  | `-- ev-emitter@1.0.3
  +-- lodash.assign@4.2.0
  +-- lodash.debounce@4.0.8
  +-- lodash.omit@4.5.0
  `-- masonry-layout@4.1.1
    +-- get-size@2.0.2
    `-- outlayer@2.1.0
      `-- fizzy-ui-utils@2.0.3
        `-- desandro-matches-selector@2.0.1
fk commented 7 years ago

👍 I can confirm that things work fine without imports-loader for masonry-layout 4.x. Hunting down the original issue and trying to find a reason for why things work now, I stumbled across this comment by the Masonry author:

Masonry v4.0 and fizzy-ui-utils v2.0.1 has a fix for this issue, using the same package names for AMD and CommonJS dependencies.

afram commented 7 years ago

Great thanks all.

I have added a deprecation notice and will remove this section of the README in due time.

Closing this issue now, please feel free to open if needed :-)