baconjs / bacon.js

Functional reactive programming library for TypeScript and JavaScript
https://baconjs.github.io
MIT License
6.47k stars 330 forks source link

Two Bacon objects from different libraries conflict. #522

Closed eguneys closed 6 years ago

eguneys commented 9 years ago

I have a library that exports a bacon stream, and it also exports Bacon as well:

My Library

module.exports.sampleStream = Bacon.fromPromise(something);

// for example I also export Bacon object
module.exports.Bacon = require('baconjs');

Now I want to use this library in another project:

My Application

var myLib = require('MyLibrary');
var Bacon = require('baconjs');
var Bacon2 = myLib.Bacon;

// faulty
myLib.sampleStream.flatMap(function(x) {
  return Bacon.fromPromise(something);
});

// correct
myLib.sampleStream.flatMap(function(x) {
  return Bacon2.fromPromise(something);
});

The problem is the first flatMap can't flatten the streams, because it doesn't recognize Bacon.fromPromise event stream. The second version works because I use the Bacon2.fromPromise which is the Bacon instance from my library.

Particularly this check fails: isObservable = (x) -> x instanceof Observable.

I don't know if I am using the node module system wrong, or this is a bug.

phadej commented 9 years ago

that is known issue in the npm package design. AFAIK there are no way to ensure that the package is present only once in the dependency tree.

with depedency diagram:

app -> lib1 -> bacon
app -> lib2 -> bacon
app -> bacon

you'll end with three Bacons, possibly of different versions too.

The only way to quickly overcome this problem is to make libraries parametric in Bacon:

// MyLibrary
module.exports = function(Bacon) {
  function foo() {}
  return {
    foo: foo
  };
}
// MyApp
var Bacon = require("baconjs");
var MyLibrary = require("mylib")(Bacon);
lautis commented 9 years ago

@eguneys are the version requirements in the library and your application compatible? If so, npm dedupe might help.

raimohanska commented 9 years ago

Should we replace instanceof checks with smthing smarter?

On 9.1.2015, at 17.31, Oleg Grenrus notifications@github.com wrote:

that is known issue in the npm package design. AFAIK there are no way to ensure that the package is present only once in the dependency tree.

with depedency diagram:

app -> lib1 -> bacon app -> lib2 -> bacon app -> bacon you'll end with three Bacons, possibly of different versions too.

The only way to quickly overcome this problem is to make libraries parametric in Bacon:

// MyLibrary module.exports = function(Bacon) { function foo() {} return { foo: foo }; } // MyApp var Bacon = require("baconjs"); var MyLibrary = require("mylib")(Bacon); — Reply to this email directly or view it on GitHub.

phadej commented 9 years ago

@raimohanska Two different Bacon's won't work together as they have different private global state. The same issue as with frames https://github.com/baconjs/bacon.js/issues/296.

Changing instanceof to something else won't help here. I guess there are no point to change, as long as Bacon has global private state.

Macil commented 9 years ago

I made BaconCast to handle converting foreign Bacon streams (and RxJS streams too) into Bacon streams usable with a local Bacon library instance. This can allow a library to accept Bacon and RxJS streams passed to it.

However, it doesn't allow the library to pass streams back to the application unless the application was also using Bacon and BaconCast, but if an application and library are going to closely coordinate like that, it makes more sense for the library to not directly depend on Bacon itself, instead only setting a peerDependency, and have the application pass a reference to its Bacon library instance into the library. (This is what BaconCast itself does.) BaconCast is useful for cases where a library uses Bacon internally, and Bacon usage is optional to the application.

kryptt commented 9 years ago

I believe marking bacon as a peer dependency in your library will have the desired effect:

"peerDependencies": { "baconjs": "^0.7.70" }

raimohanska commented 6 years ago

Peer deps are the way to go.