shama / webpack-stream

:tropical_drink: Run webpack through a stream interface
MIT License
1.39k stars 122 forks source link

Allow using webpack 5 #229

Closed snoack closed 3 years ago

snoack commented 3 years ago

This pull request relaxes the depdency on "webpack": "^4.26.1" so that webpack-stream can be used as well with webpack 5.

shama commented 3 years ago

What if this library doesn't work with webpack >= 5 without needing changes?

snoack commented 3 years ago

I tried the latest master of webpack-stream with webpack 5.3.2 (latest) in our (non-trivial) build chain. I had to do some changes on our end due to breaking changes in webpack itself, but webpack-stream seems to just work fine as-is.

shama commented 3 years ago

@snoack If it works with webpack 5 I think we should bump the version instead of relaxing the dependency. Because we don't know if it will work with webpack 6, 7, 8, etc.

snoack commented 3 years ago

I just realized that you can pass in a webpack version of your choice to webpack-stream. So it doesn't really matter that much which version webpack-stream depends on. But I started to wonder why depending on any version of webpack in the first place then? If you are using a different version than what webpack-stream depends on, you just end up with two different versions of webpack in your dependency graph.

shama commented 3 years ago

It depends on the latest major version known to work with webpack-stream of webpack. That way if someone npm i webpack-stream and tries to use the library, it just works. As opposed to potentially breaking for a period of time when/if webpack does a major version release. ^4.26.1 will install the latest 4.x version. Changing the major version typically indicates there are backwards compatible breaking changes.

You can optionally supply your own version of webpack because there are cases when the version of webpack is upgraded in webpack-stream but the user isn't ready to upgrade their webpack build. Or if someone wants to upgrade to a later version of webpack and deal with the risks of it not working, they can. In those cases, there will be 2 different versions of webpack installed but file space isn't a big penalty there IMO.

snoack commented 3 years ago

Fair enough. Though I personally think it would be a better design to make it mandatory to pass in webpack, and use peerDependencies to indicate compatible webpack versions.

I'm going to close this pull request. But note that webpack 5 has been released about a month.

shama commented 3 years ago

I've considered peerDependencies in the past but they didn't always work as expected. You could get deadlocked. Basically in order to upgrade webpack-steam you would need to update every other plugin to a compatible version of the peerDependency. Versus webpack-stream just using a version of webpack that is known to work and all the other plugins can use the other version of webpack that works for them in that mixed upgrade scenario.

Also if someone is willing to show webpack-stream is working with webpack 5 and sends a PR for it, I'll review and merge. I'm personally still using webpack 4.37 so upgrading to 5 here isn't a high priority for me atm.