-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Strawman proposal for providing Fantasyland Stream type in a separate package #675
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The overall approach here looks good, @semmel. Thanks!
My current thinking is that this could be a separate package in most-community. What do you think?
return this.stream.run(sink, scheduler) | ||
} | ||
|
||
['fantasy-land/concat'](nextStream: Stream<A>): FantasyLandStream<A> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's probably some discussion to be had on what a proper stream monoid should be. There might even be more than 1 valid monoid instance.
The way you've used continueWith
here is nice. It's like a simple switch
. I've come to believe that merge
is also a pretty good semigroup.
Any thoughts or insights on which might be the better default semigroup instance? Are there others? See my other, related comment about zero
as well.
return fantasyLand<B>(chain(fn, this.stream)) | ||
} | ||
|
||
['fantasy-land/zero']() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way I read the FL spec, @more/core-fl
would also need to implement alt
. Is that right?
It seems like you'll need to ensure that concat
+empty
are coherent, as are alt
+zero
. Have you thought about what makes sense for those in terms of coherence and their laws?
Yes, of course. But is it smart to use TypeScript for that community package then? (Could be just a matter of personal taste though). For example I could rather implement the FL wrapper in JS:
I am not particular keen of working in TypeScript, but I value the type definitions. Would it be better (e.g. aid debugging) if @most/core/dist/indes.es.js would be ES2020 and not ES5? |
Great!
It's totally up to you. You'll own the repo and the package, so if you prefer to build in JS and include .d.ts files for types (or not), that's totally cool with us 😄 . When you're ready, you can open an issue here to request membership, and we'll add you as a collaborator.
1-3 all sound fine to me.
There might be other options, but I can think of two you could consider:
I think 2 could be a bit more helpful for existing projects that are already using
Yeah, you're right that targeting a new ES version is probably best. I'm not sure when we'll get to it, but I agree it's a good idea. |
This is a proposal how one can provide @most/core-fl by rebuilding @most/core and wrapping the Stream type with a FantasylandStream type.
import @most/core
, so everything remains in TypeScript,The implementation is simple, however the typings get quite laborious
See Fantasyland support issue