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

refactor: remove store providers and restrict initial flow for EditorContainer #6167

Merged
merged 14 commits into from
Feb 3, 2024

Conversation

Flrande
Copy link
Member

@Flrande Flrande commented Jan 31, 2024

Remove the provider field from store

The original implementation of the provider was mostly considered for AFFiNE's use cases. Now, as AFFiNE no longer uses the provider mechanism of BlockSuite itself and has implemented synchronization logic separately, the provider mechanism of BlockSuite is no longer necessary. Moreover, as an editor project natively supporting Yjs, BlockSuite should not have excessive constraints on the provider. Considering the above points, the provider-related code of BlockSuite has been removed.

Standardize the editor initialization process

The original implementation of the editor's initialization process was very chaotic, mainly manifested in the following aspects:

  1. The API design of loading a Page Y.Doc is confusing. The positioning of load and trySyncFromExistingDoc is redundant, and it further leads to unclear timing of the triggering of slots.ready. load actually calls trySyncFromExistingDoc, and both load and trySyncFromExistingDoc may trigger slots.ready.

  2. The handling of the editor mounting timing is chaotic. EditorHost is the ultimate management centre of the editor, as long as EditorHost is successfully mounted, it can automatically update the Dom based on the updates of Yjs data. The basic condition for mounting EditorHost is a Page with a block of root type, and there are many situations regarding the source and timing of data when initializing the editor

    2.1. Completely manual initialization, that is, calling workspace.createPage() and page.addBlock()
    2.2. Pulling data from the provider, with no page in the workspace.
    2.3. Pulling data from the provider, with pages in the workspace but no block of root type.
    2.4. Pulling data from the provider, with pages in the workspace and a block of root type.

If it is impossible to know from the provider whether the data pull has ended, we need to manually subscribe to determine which of the situations 2.2, 2.3, and 2.4 we are in. In the original code, the above subscription operations were distributed in different levels and modules, for example, handling the situation 2.2 in the entry function, and handling the situation 2.4 in the EditorContainer.

After the restructuring:

  1. Page no longer exposes trySyncFromExistingDoc, only exposes load, and slots.ready is only triggered in load. ready only ensures that the blocks tree is synchronized with Yjs data when calling page.load(), and does not guarantee that the data meets the requirements for mounting EditorHost.

  2. The mounting requirements of EditorContainer are aligned with EditorHost, requiring a Page with a block of root type. Before mounting EditorContainer, it is necessary to ensure that the data in the Page meets the mounting requirements.

Basic initialization process after restructuring:

For 2.1, the usage is not much different from before.

const workspace = new Workspace(options);
const page = workspace.createPage({ id: 'page:home' });
await page.load(() => {
  const pageBlockId = page.addBlock('affine:page', {
      title: new Text(),
    });
  });
page.resetHistory();
mountEditorContainer();

For 2.2, 2.3, 2.4, it's like gathering the previously scattered handling code together.

const workspace = new Workspace(options);
const provider = setupProvider(workspace);
provider.connncet();
// wait for first page
const firstPageId = await new Promise<string>(resolve =>
  workspace.slots.pageAdded.once(resolve)
);
const page = workspace.getPage(firstPageId);
assertExists(page);
await page.load();
if (!page.root) {
  // wait for root block
  await new Promise(resolve => page.slots.rootAdded.once(resolve));
}
page.resetHistory();
mountEditorContainer();

If we can know the timing of synchronization completion from the provider, and we know that the synchronized block structure meets the requirements, the code can be simplified a lot.

const workspace = new Workspace(options);
const provider = setupProvider(workspace);
await provider.sync();
const firstPage = workspace.pages.values().next().value;
assertExists(page);
assertExists(page.root);
await page.load();
page.resetHistory();
mountEditorContainer();

Copy link

vercel bot commented Jan 31, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
blocksuite ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 2, 2024 3:38pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
blocksuite-docs ⬜️ Ignored (Inspect) Visit Preview Feb 2, 2024 3:38pm

@Flrande Flrande changed the title refactor: remove store providers and refactor playground refactor: remove store providers and restrict initial flow for EditorContainer Feb 2, 2024
Copy link
Member

@doodlewind doodlewind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you clarify the API changes in PR description? Thanks!

@Flrande
Copy link
Member Author

Flrande commented Feb 2, 2024

Could you clarify the API changes in PR description? Thanks!

done

Copy link
Member

@doodlewind doodlewind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the impressive refactoring!

@doodlewind doodlewind merged commit f239fd1 into master Feb 3, 2024
19 checks passed
@doodlewind doodlewind deleted the flrande/refactor/playground-store-0131 branch February 3, 2024 02:42
@Flrande
Copy link
Member Author

Flrande commented Feb 3, 2024

This PR may have an impact on the encapsulation of the AFFiNE side, as it constrains the mounting requirements of the EditorContainer. cc @pengx17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notable Major improvement worth emphasizing
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants