xstream has an old xstream as a dependency so crashes in latest cycle run
Closed this issue · 21 comments
Current have a explicit dep on xstream 5.0.0 which needs to be removed
Very true. Thanks
So after a little research, I found that peerDependencies seems not to be the perfect solution for it. (cf. https://codingwithspike.wordpress.com/2016/01/21/dealing-with-the-deprecation-of-peerdependencies-in-npm-3/)
So I guess (for the moment) I'll stick with that version of the package.json (if you install xstream in your app, it's your version that will be used).
Looking at Cycle Run (Unified version) code, there are no peerDependencies and a fixed version of xstream declared, so I guess that not really an issue https://github.com/cyclejs/cyclejs/blob/unified/run/package.json#L32
The discussion is open though ...
So I guess (for the moment) I'll stick with that version of the package.json (if you install xstream in your app, it's your version that will be used).
Is that definitely true? Worse case is different modules use different versions which would blow up if refs passed between them.
Given the rules of semver and the major versions are 5 and 10 respectively I find it unlikely the npm would use 10 for both deps.
If both versions are included that will bloat the bundle - unless some form of aggressive tree shaking is done. I'll check my bundle.
Oh, do you have any plans to go to cycle unified? I don;t see any way a library / driver author can support both versions though
That's an interesting discussion :)
I think I need a little more inputs to fully understand what can be the advantages and drawbacks of each solution (with or without peerDependencies) before I can make a choice.
Another solution I can imagine is having a larger version support for xstream (* for example, and let the tests detect if something breaks ...). I am pretty new to lib authoring and I have to admit I am not sure at all what's best.
I didn't tested it yet, but I think this driver is already compatible with Cycle Unified as it is using xstream under the hood (this assertion needs testing of course :) )
Intersting is one work - LOL
Yes, it's needs a good understanding of dependency management and I'm afraid I've skipped over it as I tend to feel life is too short. :)
Would other cycle libs / drivers be useful references. I'm sure Tylor / Widdeshin/ Staltz could shed some light on the matter.
You might well be right about Unified - except I think you (we) need to import a different run module? Something else to add to my TODOs.
https://github.com/cyclejs/cyclejs/blob/master/history/package.json#L16-L37
In this only direct deps are listed. xstream etc are just dev deps. Thus the user is expected to include xstream
So question is what if you depend on a specific version of xstream etc?
When using code used "xstream": "^10.9.0" it completely fails to run in the cyclejs code.
The reason being the auth0 sink returned from protect() does not have the correct format and .subscribe() is undefined.
I rebuilt locally with latest xstream and all is well. I also updated lock while I was at it though it satisifes the specified version anyway
Oh, the test pass fine with "xstream": "^10.9.0. But they also pass when cyclejs run() fails as there's no test for using in cyclejs. I'm not sure how best to add a test for that.
Not sure I have understood what happens here. What has changed in xstream since version 10.9? Is seems the subscribe has been around for quite some time (since version 6.3.0 to be precise).
Add functional test using cycle would be very interesting indeed, I might dig into this when I find time to do so.
The published package uses XStream 5.x.x and when latest cyclejs tries to subscribe to the auth0 sink stream using zxstream 10.x.x it crashes. as it can;t find subscribe.
Just build something that uses cyclejs_auth0 using latest cycle and xstream and call protect()
I've no idea why but 5 - 10 is a lot of breaking changes ;)
@SteveALee oh that's right the dependency is xstream 5.0 :/
Is still don't know how to properly deal with those peerDependencies but as I don't really need a specific xstream version, it might be a good solution (as you suggested) to add xstream as a dev dep then ... ?
xstream already is a dep.
Perhaps ask on Gitter?
@SteveALee as a dev dependency :)
So I figured the problem should be with the ^ I was using to declare the xstream dependency. I updated it to a wider range (>). I think it will fix your problem as:
- you first install
@cycle(so it also install the rightxstreamversion) - latter on you install
cyclejs-auth0(and it will keep your installed version of xstream as it is compatible)
Will this work for you?
Possibly :) Will try tomorrow. Thanks
One thought the order of installs is indeterminate. I found npm will sort the order of dependencies to be alphabeitical (I think)
What do other packages do that also include xstream?
Works a treat thanks!
Sorry - exactly the same problem with 4.0.0 :( I did a clean install this time.
@SteveALee oh you know what, my mistake I totally forgot to publish the new version on npm. Sorry about that!
I did the test, with a fresh project and an install of version 4.0.0 it installs xstream v5.3.6
now with version 4.1.0 it directly installs the last version of xstream.
Running npm update might work for you :)
I confirm is good now!