-
Notifications
You must be signed in to change notification settings - Fork 183
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
I have forked this library, let's discuss if we should join efforts or not #211
Comments
Thank you. |
Funny, pretty much on the same day you started your fork I also started a rewrite after getting frustrated with some of the issues you mention. I also opted for TypeScript, and added support for communicating with webworkers instead of iframes only, among other new features. https://github.com/alesgenova/post-me Unfortunately this issue wasn't up at the time I was working on it (Nov 27th-29th), would have been nice to collaborate |
hey @alesgenova yours is a really interesting one, maybe we can make our libs a single one? It's interesting to think that we can conceive it as a wrapper on top of |
@franleplant Yeah I think it could be a good to merge the ideas we had in our respective implementations into a single better library. Maybe let's have a chat about it in the next few days. My email is my github username at gmail. |
I am looking at rewriting this library in typescript as well, and ibridge is pretty close to what I had planned. My only objection and reason to avoid using it is your removal of the .call() method. Yes, underneath it is doing events, but wrapping it up to look and work like a promise-based RPC is just a nice thing to have! I suppose my love for it is due to putting a layer around Postmate with my own classes that turns it into an actual RPC mechanism, so perhaps instead of ditching it entirely it would be better to more fully develop the concept? |
Ibridge already does that by using Emittery with .once and the rewrite of
ibridge i havent merged yet embraces even more that concept, Im looking for
contributors
…On Thu, 11 Mar 2021 at 14:45 HyperCharlie ***@***.***> wrote:
I am looking at rewriting this library in typescript as well, and ibridge
is pretty close to what I had planned. My only objection and reason to
avoid using it is your removal of the .call() method. Yes, underneath it is
doing events, but wrapping it up to look and work like a promise-based RPC
is just a nice thing to have! I suppose my love for it is due to putting a
layer around Postmate with my own classes that turns it into an actual RPC
mechanism, so perhaps instead of ditching it entirely it would be better to
more fully develop the concept?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#211 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOEIJ7X7YFS5IXUOQBSN7DTDDXQXANCNFSM4UJIOWWQ>
.
|
why not collaborating to this one instead ? did you ever try to create a pull request here ? |
guys, found another pretty interesting implementation and still very active. https://github.com/Aaronius/penpal |
What Is the issue?
Hi guys! Thank you so much for this library it is awesome.
Unfortunately I found several problems while using it that prompted me to fork it and do major
overhaul of the code and I wanted to reach out to you so that I can explain why I did the things I did
and maybe discuss if it is worth it to plan a potential merger down the road, Im super open to that.
Find the fork here https://github.com/franleplant/ibridge I already published as ibridge in npm.
Also, I hope I did the right steps to give you guys correct attributions, if you find something wrong please let me know and I will fix it promptly.
Problems and solutions I found with postmate that lead me to build ibridge
.get
lacks proper return type support.The idea of
.get
is really good but the implementation was not ideal..get
is a way of calling model functionsin the child and retrieving values, but what happens if the model throws an error? this becomes even more important when dealing with promises, let's say fetching stuff, having to treat the errors thrown in the child functions specially so that they can fit the "always resolving" promise paradigm in
.get
was awkward and problematic.This makes
.get
a stub of what it really can be.With simple tweaks we can make it reject in the parent if it was rejected in the child.
Additionally I removed the capability of model functions to be attributes, for me it does not make sense. Im open to other ideas though.
Related issue #94
call
is just a weird way of sending eventsIntuitively when going through the lib's docs it might appear that
call
is a way of remotely calling model functions in the child and getting the return values, but it is not at all like that. Most of the things can be resolved with the improved.get
I described above.If you want to call a function in the child without caring about the return values then you still can use
.get
BUT I do see the value of allowing freely emitting of events between parent and child and that is why I added
emitToChild
andemitToParent
andon
so that if the lib consumer wants to do something outside the main happy path which isget
then they do not need to revert topostMessage
.BONUS: I make both child and parent be event emitters for a better composition of event.
model[modelKey]
is just to basicUsing this lib I found myself wanting to be able to use
lodash paths
instead of model keys when using.get
,this allows for a better structuring of more complex models. So I added support for
parent.get('some.deep.lodash.path')
.I am a big fan of typescript and I avoid touching pure javascript, so I migrated it entirely to typescript and it is awesome,
we get some cool capabilities of making the child be generic over the model type.
Another thing I really found myself patching around was the lack of
model context
. So I added the capability ofsetting up a context in the
child
that will be passed asthis
to all the model functions. We use that internallyto store references to apis such as socketio. Another apis for these sort of thing might be better, Im open to ideas.
On postmate there are a bunch of native event listeners setup on and off.
I found that I can get away with a single one (called the dispatcher) that will perform some
verification on the event and then emit internally a higher level event (using an Event Emitter library) which
abstract away a lot of the complexity of verifying, parsing, and the meta structure of the payload that is unwrapped
and used in the higher level events (such as
get/resolve
get/reject
, etc).This has worked really well, abstracting a lot of complexity from the rest of logic in the class
I didn't like much this intermediate classes that are used to init the real classes, plus returning a promise from class constructors is kind of problem, additionally, by moving this logic inside the main classes we can
fixed maxHandshakeRequests is not configurable #195
removed support for older browsers
I did that, but I believe is easy to go back on that and I am completely open to that.
I think that the end result is really nice, it has docs, it has types, I am already using it at work.
I am open to plan a merger, or I am also open to contribute some parts of that lib into this one and keep them separate the.
Thanks!
Fran
The text was updated successfully, but these errors were encountered: