A draft for discussing binder API options. Never merge.#5
A draft for discussing binder API options. Never merge.#5dstanesc wants to merge 25 commits intobinding-property-parityfrom
Conversation
…layground Main backmerge
… as coarser and unified listening mechanism. Creates better impedance match with the SharedTree visitor based delta processing. Domain adaptes are instantiations of the `DeltaVisitor` interface adapting the visitor callbacks to domain specific APIs. Path filtering becomes optional and possibly an adapter detail.
…layground Backmerge binding-property-parity
interaction granularity
…layground Backmerge binding-property-parity branch
…layground Backmerge latest binding-property-parity
Prototype more binding formalisms: NodeResolver, ChangeCategory, ChangeBinder, BatchBinder
| unsubscribeFromChildrenChange(); | ||
| }; | ||
| // assert(eventName === "changing", "unexpected eventName"); | ||
| if (eventName === "changing") { |
There was a problem hiding this comment.
a better pattern here is to use a switch statement. Then in the default case call unreachableCase. This allows the compiler to give you an error if you are missing a case. Otherwise these changes look great.
There was a problem hiding this comment.
I know you marked this never merge, but with that fix, this subset of the PR would be a great change to merge. Seems quite useful!
There was a problem hiding this comment.
For sure the suggested is the right pattern. Editable-tree changes were more like a quick & dirty support for the higher-level binding ideas wanted to verify and validate with the community. Definitely, if appealing to the team I could make a dedicated PR with the additional subtreeChanging event in EditableTree.
| * Provides events that indexes can subscribe to | ||
| */ | ||
| private readonly indexEventEmitter = createEmitter<IndexEvents<TChange>>(); | ||
| public readonly indexEventEmitter = createEmitter<IndexEvents<TChange>>(); |
There was a problem hiding this comment.
making this public would allow users of this class to trigger events (not just subscribe to them). I'm not sure what the usecase here is, but the general pattern is we don't expose emitters other than as ISubscribable.
Also these events are for the tree as a whole and not the specific view/branch of the tree you are interacting with. Can your usecase be solved with the new "afterBatch" event accessed via ISharedTreeBranch.events?
There was a problem hiding this comment.
This change is obsolete. Leaked accidentally from earlier versions of a quick & dirty prototype which required newLocalState before afterBatch was available. I confirm that current version uses afterBatch to detect completion.
- Appears that listener resultType cannot be inferred in the on(...) calls. Likely unfeasible to evolve the events API. Bridged the clash w/ void events w/ !== undefined assertions. - The tsdoc build seems to fail on the newly introduced listeners w/ return values
2 participants
A draft for discussing binder API options. Never merge.