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

[Enhancement] CollaborationService should listen to proper events #5

Closed
2 tasks
situ2001 opened this issue Aug 6, 2022 · 8 comments
Closed
2 tasks
Assignees

Comments

@situ2001
Copy link
Owner

situ2001 commented Aug 6, 2022

目前创建TextModel与YText的binding的方法如下

通过监听用户切换EditorGroup的标签或者切换EditorGroup,即监听事件EditorActiveResourceStateChangedEvent,该事件发生后就把两者给绑定起来,顺手把当前的Editor给加进到这个binding上。销毁binding的方法就是监听EditorGroupCloseEvent,当对应的uri没有被打开,就会销毁掉这个binding...

虽然能工作,但有可能会出bug

今天在读源码的时候,看到了事件EditorDocumentModelCreationEvent,对应的是TextModel的创建

this._modelReferenceManager.onInstanceCreated((model) => {
this.eventBus.fire(
new EditorDocumentModelCreationEvent({
uri: model.uri,
languageId: model.languageId,
eol: model.eol,
encoding: model.encoding,
content: model.getText(),
readonly: model.readonly,
versionId: model.getMonacoModel().getVersionId(),
}),
);
});

读的时候,还找到了这个事件,对应的是TextModel的销毁

this.onDispose(() => {
this.eventBus.fire(new EditorDocumentModelRemovalEvent(this.uri));
});

由于TextModel与YText是绑定在一起的,为何不在EditorDocumentModel创建之后就马上绑上YText,然后直到EditorDocumentModel销毁的时候再销毁这个binding?

TODO

  • 将binding的创建与销毁移动到事件EditorDocumentModelCreationEventEditorDocumentModelRemovalEvent
  • 处理Editor添加到binding或从binding中移除,应监听事件EditorGroupCloseEventEditorGroupOpenEvent
@situ2001 situ2001 self-assigned this Aug 6, 2022
@situ2001 situ2001 changed the title [Improvement] Create binding when on event EditorDocumentModelCreation [Improvement] Listening to proper events in collaboration service Aug 6, 2022
@situ2001 situ2001 changed the title [Improvement] Listening to proper events in collaboration service [Improvement] Collaboration service should listen to proper events Aug 6, 2022
@situ2001
Copy link
Owner Author

situ2001 commented Aug 6, 2022

这个改造应该可行,并且比较重要,先把 #3 的事情延后一下

@situ2001 situ2001 mentioned this issue Aug 6, 2022
8 tasks
@situ2001 situ2001 changed the title [Improvement] Collaboration service should listen to proper events [Improvement] CollaborationService should listen to proper events Aug 6, 2022
@situ2001
Copy link
Owner Author

situ2001 commented Aug 6, 2022

实际用起来是可以的,过一会把测试写上去

@situ2001 situ2001 changed the title [Improvement] CollaborationService should listen to proper events [Enhancement] CollaborationService should listen to proper events Aug 7, 2022
@situ2001
Copy link
Owner Author

situ2001 commented Aug 7, 2022

但是又有一个新问题了,这会在删除文件的时候,有可能不起作用(发现默认情况下,DocModel不会立即被移除)

@situ2001
Copy link
Owner Author

situ2001 commented Aug 7, 2022

查看源码得知,DocModelRef的移除是做了Debounce的,默认3s,但即使设置为0s也无法避免删除文件后不关闭打开的显示为(已删除)的tab的同时再次创建同名文件,无法正常创建binding的问题。

因为该ref的移除会在引用数为0且当前执行栈空后(即使setTimeout超时为0),从task queue中取出来执行,无法干预“删除文件后不关闭打开的显示为(已删除)的tab的同时再次创建同名文件”的这个行为,他用回的依旧是同一个ref

@situ2001
Copy link
Owner Author

situ2001 commented Aug 7, 2022

当然代码还有一处地方用到了ref但没有把它dispose掉,这也会使ref无法被移除(removal)

不过,即使改回来,也没有太大的作用,见上条comment

@situ2001
Copy link
Owner Author

situ2001 commented Aug 7, 2022

查看源码得知,DocModelRef的移除是做了Debounce的,默认3s,但即使设置为0s也无法避免删除文件后不关闭打开的显示为(已删除)的tab的同时再次创建同名文件,无法正常创建binding的问题。

因为该ref的移除会在引用数为0且当前执行栈空后(即使setTimeout超时为0),从task queue中取出来执行,无法干预“删除文件后不关闭打开的显示为(已删除)的tab的同时再次创建同名文件”的这个行为,他用回的依旧是同一个ref

哦,之前的那种方法也会引起这样的bug...

@situ2001
Copy link
Owner Author

situ2001 commented Aug 7, 2022

查看源码得知,DocModelRef的移除是做了Debounce的,默认3s,但即使设置为0s也无法避免删除文件后不关闭打开的显示为(已删除)的tab的同时再次创建同名文件,无法正常创建binding的问题。

因为该ref的移除会在引用数为0且当前执行栈空后(即使setTimeout超时为0),从task queue中取出来执行,无法干预“删除文件后不关闭打开的显示为(已删除)的tab的同时再次创建同名文件”的这个行为,他用回的依旧是同一个ref

这个也许可以把防抖时间设为0。然后加入行为:当文件被删除后,强行干掉这个已删除文件的tab?(这么做,有点诡异)

@situ2001
Copy link
Owner Author

situ2001 commented Aug 8, 2022

既然DocModel在本地是引用计数的,删除文件这类操作,在该模块上的设定是:服务端监听删除事件,然后从yMap中删掉这个YText,新建文件的时候,客户端再去请求新建YText,最后客户端通过监听YMap的event来获取这个新建的YText

那么好办了,本地留好binding map,当action === add出现,要么是第一次绑上YText和TextModel。也有可能是TextModel和binding还在,只是YText发生了改变。那么我们只要把binding上的YText给换掉不就行了?

@situ2001 situ2001 closed this as completed Aug 8, 2022
@situ2001 situ2001 reopened this Aug 8, 2022
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

Successfully merging a pull request may close this issue.

1 participant