Skip to content

feat: use yjs as the source-of-truth #440

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Aug 11, 2023
Merged

feat: use yjs as the source-of-truth #440

merged 8 commits into from
Aug 11, 2023

Conversation

lihebi
Copy link
Collaborator

@lihebi lihebi commented Aug 10, 2023 •
edited
Loading

This is a major change. Close #331

Before

Previous architecture (detailed in #205) is centered around the browser (yjs <-> browser <-> DB), which is problematic because:

  1. we have two (potential inconsistent) sources of data (yjs and DB)
  2. there can be multiple users/browsers. It's complex to decide which user/browser should push to DB

After

Now we are changing to Yjs-centered (browser <-> Yjs <-> DB). Yjs is the single source-of-truth.

Details

This PR removes a lot of syncing logic/functions from front-end, notably those addPod, loadRepo, setPodGeo reducers.

TODOs:

  • Better indicator to show the syncing status (e.g., synced, offline), prompt user unsynced changes when user leave the page.
  • When a user gets disconnected, edits can still be made, but these new edits might be lost. This is a bug. We need to figure out a graceful way to re-sync when Yjs WS re-connects.

@lihebi lihebi marked this pull request as ready for review August 10, 2023 20:45
@lihebi lihebi mentioned this pull request Aug 11, 2023
Merged
@lihebi
Copy link
Collaborator Author

lihebi commented Aug 11, 2023 •
edited
Loading

This feature is roughly usable. I'm merging this so that people can work on top of this, and avoid merge conflicts, as this PR is too large and touches everything.

Remaining problems: state.pods is now empty. So the following things break and need fixes in the follow-up PRs:

  • pod.content access, e.g., runtime, parser
  • pod.result access, e.g., display result, stdout/stderr, images
  • export .ipynb and markdown
  • copy & paste

@lihebi
Copy link
Collaborator Author

lihebi commented Aug 11, 2023

Note: DB schema changed. You need to apply the migration and recreate your docker stack.

@lihebi lihebi merged commit fb5ec9e into codepod-io:main Aug 11, 2023
@lihebi lihebi mentioned this pull request Aug 15, 2023
Merged
4 tasks
lihebi
lihebi commented Aug 19, 2023
View reviewed changes
Copy link
Collaborator Author

@lihebi lihebi left a comment

Choose a reason for hiding this comment

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

A regression bug in two places.

ui/src/lib/store/runtimeSlice.tsx
Comment on lines -26 to +27
let siblings = get().node2children.get(parentId) || [];
const siblings = nodes.filter((node) => node.parentNode === parentId);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

bug: previous siblings are IDs, new siblings are Nodes.

ui/src/lib/store/runtimeSlice.tsx
Comment on lines -377 to +380
let children = get().node2children.get(id);
const children = nodes.filter((n) => n.parentNode === id);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

bug: ditto

@lihebi lihebi mentioned this pull request Aug 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Yjs as the source of truth
1 participant
@lihebi