Skip to content
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

Proposal for new tangle interface #39

Open
christian-bromann opened this issue Jul 14, 2022 · 1 comment
Open

Proposal for new tangle interface #39

christian-bromann opened this issue Jul 14, 2022 · 1 comment
Assignees

Comments

@christian-bromann
Copy link
Contributor

After using tangle now in multiple VSCode projects I've run into several patterns where the current interface design fails. Therefor I would like to suggest some (breaking) changes to the package:

Usage of RxJS

RxJS has a steep learning curve and hard to get for people not familiar with it. I personally struggle fixing some of the reported issues and wouldn't wonder if others do so too. I am not sure if there is a clear need for RxJS for the tangle usecase, there are other ways to accomplish the same.
I'ld suggest to simplify the code base and remove RxJS as dependency and rewrite the functionality using bare JS primitives.

State Updates

Currently tangle's broadcast method requires the whole state to be submitted, it would be nice if updates can be made on single object properties. Maybe we can have a broadcast and broadcastAll?

That said, currently a state can be everything which turns out to be not useful in reality. It seems weird to juggle with multiple client instances because you have multiple states. It seems more straightforward to say a state must be a Map which can contain multiple state properties.

Every state listener is triggered whenever any state property changes. This comes from the fact mentioned above as the current implementation interprets a state as one object and can't determine if just parts of the state has changed.
With the suggestion above we should ensure that listening on a state property only triggers the listener when that particular state property has changed, e.g.:

const ch = new Channel('foobar', {
    counter: 0,
    anotherProperty: true
});

bus.listen('counter', (counter) => console.log(`Counter update: ${counter}`))
bus.broadcast({ anotherProperty: false }) // <--- does not trigger listener above

This causes a lot of re-renders in the Marquee extension.

Event vs. State

In the current implementation event and state is handled separately but not when it comes to types. For example:

interface StateType {
    counter: number
}

const ch = new Channel<StateType>('foobar', {
    counter: 0
});

bus.emit('foobar', 'barfoo') // type issue as `foobar` is not part of `StateType`

I suggest to not expect any types for event methods except you pass in a type, e.g.:

interface Events {
    foobar: string
}
bus.emit<Events>('foobar', 'barfoo')
@sourishkrout
Copy link
Member

sourishkrout commented Jul 15, 2022

rxjs or not

While I'm still convinced Observables are superior for these kinds of problems, I hear you on the learning curve. In a lot of ways, I don't want to make this a religious functional vs imperative programming contention point. If you have a plan in your head how a low-level rewrite would look like, I don't want to stand in the way. Probably just have to figure out some rough estimates and when we want to take the time for it.

state updates

This relates to my response here: #38 (comment)

Event vs. State

Yeah, that makes a lot of sense. I do think there's a benefit in having stronger typing & separation between events and shared state. In a lot of ways, tangle kinda treats them separately on a lower-level but not all the way through typing.
I'm not so sure about the Map-approach since it would water-down the structural typing of app state from a data structure to a blob of data. My tangle vision (not too attached to it) for app state was to ideally have a sharable type between clients per channel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants