Generated bundle includes dependencies about react-elastic-carousel

[mAAdhaTTah]

Sure! I could probs take a look at this tomorrow.

[sag1v]

@mAAdhaTTah But what if the consumer doesn't use these two libraries? I'm not sure that each tertiary dependency should be a peer-dependency, its exhausting to install them manually.

[mAAdhaTTah]

@sag1v They shouldn't be peerDeps, just regular deps, like React & SC are.

[sag1v]

@sag1v They shouldn't be peerDeps, just regular deps, like React & SC are.

Well, react and SC are peerDeps.

[sag1v]

@mAAdhaTTah I just remembered that rollup will bundle it ALL together via the commonjs plugin..

[mAAdhaTTah]

@sag1v Yeah, that's the cause. I believe if you move them to deps, the external plugin will make sure they're not bundled.

[sag1v]

@mAAdhaTTah nah, external() will ignore all peerDeps.
If you see in package.json, the only entry in dependencies is react-swipeable (which is also included in the bundle), my mistake was not including both resize-observer and classnames there as well, but i see that they are included in the bundle anyway.
I suspect that rollup-plugin-node-resolve and rollup-plugin-commonjs are forcing them to be part of the bundle (maybe they check for imports to decide).

I still think the end results is good (though achieved in a wrong way), if you look at the peerDeps array, you notice that none of them are bundled, but requires to get installed by the consumer. I don't think making consumers explicitly install classnames and resize-observer-polyfill is a good thing.

BTW, i just realised that we use classnames only in one place, i think of ditching it anyway.

[mAAdhaTTah]

@sag1v I agree that consumers should not need to install those libs to use the library. I disagree that the only alternative is to bundle non-peer-deps into the library. That's the behavior of the plugin you're using, but you could e.g. switch to auto-external or even using rollup's external to do it manually. Ultimately, my goal of opening this ticket is to remove those from the bundle so my installation can dedupe those dependencies appropriately.

[mAAdhaTTah]

Oh, to be clear, it's the behavior of rollup-plugin-peer-deps-external.

[sag1v]

@mAAdhaTTah I'm not sure i understand.

Whats do you think should be manually added to the external array? And what would happen if consumers wont include it manually?

For example, if we put resize-observer-polyfill in external but the consumer is not using it nor including it in their project, then where should it come from?

[mAAdhaTTah]

@sag1v If resize-observer-polyfill is in this library's package.json as a dependency, npm or yarn will install it automatically for them.

[sag1v]

Hmm something like this plugin?
https://www.npmjs.com/package/rollup-plugin-exclude-dependencies-from-bundle

[mAAdhaTTah]

Yeah that would work.

[sag1v]

I would welcome a PR 😁
Will be nice if you check that its actually working well for you...

[sag1v]

Available as of version 0.7.1
Thank you @mAAdhaTTah !

[mAAdhaTTah]

@sag1v Great, thank you!