Dependencies disparities & cleanup

[ngryman]

Hi guys,

By inspecting the result of npm list on my project, I've seen some dependencies disparities among some Ampersand modules.

First, it seems that some devDependencies are referenced in dependencies. They end up being bundled by browserify, increasing bundle's size. Removing them would lighten bundles quite a lot:

• ampersand-sync@2.0.3:
  • tap-spec
• ampersand-sync@2.0.3
  • xhr@1.16.0
    • global@2.0.7
      • min-document@0.2.8
        • tape
• ampersand-router@1.0.4
  • jshint
• ampersand-view@7.1.1
  • key-tree-store@1.2.0
    • tape

I'll create an issue in third party projects, and keep you posted here, so you can update the dependency.

Then, would it be possible to normalize common dependencies versions? Browserify includes duplicate dependencies because version is not the same:

• underscore
  • 1.6.0
    • ampersand-collection-view@1.1.1
    • ampersand-sync@2.0.3
    • ampersand-router@1.0.4
    • ampersand-view@7.1.1
  • 1.7.0
    • ampersand-model@4.0.2
• backbone-events-standalone
  • 0.2.2
    • ampersand-state@4.3.10 (ampersand-model@4.0.2, ampersand-view@7.1.1)
    • ampersand-router@1.0.4
  • 0.2.4
    • ampersand-collection-view@1.1.1
• key-tree-store
  • 1.2.0
    • ampersand-state@4.3.10 (ampersand-model@4.0.2, ampersand-view@7.1.1)
  • 0.2.1
    • ampersand-dom-bindings@3.1.1 (ampersand-view@7.1.1)

Thanks!

[ngryman]

xhr has corrected the issue and published v1.16.1 (naugtur/xhr#48).

[lukekarrys]

@ngryman I took care of ampersand-router and ampersand-sync. Thanks for the issue!

It's also worth noting that browserify wont bundle anything unless it is actually included with require. So in the the ampersand-router example, jshint wouldn't end up in the browserify bundle since it doesn't do require('jshint').

But it would be an issue for things in your second example. Like ampersand-state and ampersand-dom-bindings would each bundle their own copy of key-tree-store since they having differing major versions.

Apologies if this is already what you were saying, I just wanted to clarify to avoid confusion. 😄

[ngryman]

@lukekarrys My bad, devDependencies are not bundled 😄 So this is not as critical as I thought.

Currently, I just end up with 2 different underscore, backbone-events-standalone and key-tree-store.
Would you like pull requests for this?

[lukekarrys]

You can also run npm dedupe in your project and the extra underscore and backbone-events-standalone modules should go away if you build your browserify bundle again. So I don't see much need to bump backbone-events-standalone and underscore versions since they are so close together, and would be deduped.

Seems like it would be a good idea to get ampersand-state on key-tree-store@1.2.0 though.

Also, we're already planning to move away from underscore so for now that would probably be extra work that wont be used for much longer.

[ngryman]

Nice ! Didn't knew about npm dedupe.

It works well for underscore and backbone-events-standalone, but fails for matches-selector and key-tree-store because they have different major versions I guess:

npm WARN unavoidable conflict matches-selector { '/xxx/node_modules/ampersand-view/node_modules/events-mixin/node_modules/delegate-events/node_modules/closest/node_modules/matches-selector': '0.0.1',
npm WARN unavoidable conflict   '/xxx/node_modules/ampersand-view/node_modules/matches-selector': '1.0.0' }
npm WARN unavoidable conflict Not de-duplicating
npm WARN unavoidable conflict key-tree-store { '/xxx/node_modules/ampersand-state/node_modules/key-tree-store': '0.1.2',
npm WARN unavoidable conflict   '/xxx/node_modules/ampersand-view/node_modules/ampersand-dom-bindings/node_modules/key-tree-store': '1.2.0' }
npm WARN unavoidable conflict Not de-duplicating

So it would be very nice to bump older versions.

On a side note, you should mention npm dedupe somewhere in the docs in the short term.
Later, I think shared dependencies versions should always be aligned across all ampersand modules, in order to get optimal bundle sizes out of the box :)

Thanks a lot!

[ngryman]

And yeah, I've seen that you are moving away from underscore, which is 👍

[bear]

closing this per the discussion in AmpersandJS/ampersand-state#80 (comment)