-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Improvement: High Performance MobX Integration for Pages ✈︎ #3397
Conversation
web/pages/[workspaceSlug]/projects/[projectId]/pages/[pageId].tsx
Outdated
Show resolved
Hide resolved
…issueEmbed` hook
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@henit-chobisa everything else looks good, just a couple of minor changes... and could you also resolve the conflict the way we discussed yesterday?
this is beautiful, thanks @henit-chobisa for the detailing this. 🍰 |
A pleasure to pull this off, one of my most fun and enjoyable pull requests! ✌🏼 |
* fix: removed parameters `workspace`, `project` & `id` from the patch calls * feat: modified components to work with new pages hooks * feat: modified stores * feat: modified initial component * feat: component implementation changes * feat: store implementation * refactor pages store * feat: updated page store to perform async operations faster * fix: added types for archive and restore pages * feat: implemented archive and restore pages * fix: page creating twice when form submit * feat: updated create-page-modal * feat: updated page form and delete page modal * fix: create page modal not updating isSubmitted prop * feat: list items and list view refactored for pages * feat: refactored project-page-store for inserting computed pagesids * chore: renamed project pages hook * feat: added favourite pages implementation * fix: implemented store for archived pages * fix: project page store for recent pages * fix: issue suggestions breaking pages * fix: issue embeds and suggestions breaking * feat: implemented page store and project page store in page editor * chore: lock file changes * fix: modified page details header to catch mobx updates instead of swr calls * fix: modified usePage hook to fetch page details when reloaded directly on page * fix: fixed deleting pages * fix: removed render on props changed * feat: implemented page store inside page details * fix: role change in pages archives * fix: rerending of pages on tab change * fix: reimplementation of peek overview inside pages * chore: typo fixes * fix: issue suggestion widget selecting wrong issues on click * feat: added labels in pages * fix: deepsource errors fixed * fix: build errors * fix: review comments * fix: removed swr hooks from the `usePage` store hook and refactored `issueEmbed` hook * fix: resolved reviewed comments --------- Co-authored-by: Rahul R <[email protected]>
* fix: removed parameters `workspace`, `project` & `id` from the patch calls * feat: modified components to work with new pages hooks * feat: modified stores * feat: modified initial component * feat: component implementation changes * feat: store implementation * refactor pages store * feat: updated page store to perform async operations faster * fix: added types for archive and restore pages * feat: implemented archive and restore pages * fix: page creating twice when form submit * feat: updated create-page-modal * feat: updated page form and delete page modal * fix: create page modal not updating isSubmitted prop * feat: list items and list view refactored for pages * feat: refactored project-page-store for inserting computed pagesids * chore: renamed project pages hook * feat: added favourite pages implementation * fix: implemented store for archived pages * fix: project page store for recent pages * fix: issue suggestions breaking pages * fix: issue embeds and suggestions breaking * feat: implemented page store and project page store in page editor * chore: lock file changes * fix: modified page details header to catch mobx updates instead of swr calls * fix: modified usePage hook to fetch page details when reloaded directly on page * fix: fixed deleting pages * fix: removed render on props changed * feat: implemented page store inside page details * fix: role change in pages archives * fix: rerending of pages on tab change * fix: reimplementation of peek overview inside pages * chore: typo fixes * fix: issue suggestion widget selecting wrong issues on click * feat: added labels in pages * fix: deepsource errors fixed * fix: build errors * fix: review comments * fix: removed swr hooks from the `usePage` store hook and refactored `issueEmbed` hook * fix: resolved reviewed comments --------- Co-authored-by: Rahul R <[email protected]>
High Performance MobX Integration for Plane Pages
What was the performance issues we were facing in pages
Initially with SWR in plane pages, we were facing a lot's of performance related issues like taking time in archiving and deleting the pages, data loss issues because of hook not able to load the data, inconsistent api calls and mutation issues, to fix all of the state management issues we chose to go forward with MobX inside pages, which we were using previously inside other features like issues. This PR introduces a new way of implementing MobX store inside pages.
What architecture we have opted for creating the new performant plane pages
As discussed by @sriramveeraghanta and @rahulramesha, in our previous approaches we used to have a
projectAssetStore
and aglobalAssetStore
. The global asset store used to store all the asset irrespective of the project they are coming from, which the purpose of theprojectAssetStore
would be to store the mapping of which asset belongs to which project. Hence when you would want to pick edit one of the asset you can get it with the help of both of these stores and these are the global store is the single source of truth.In the current approach, we are trying to forming a hierarchical structure, where we are not dealing with the data directly but we are dealing with the class instances, which will only going to be added or deleted but are never going to modify and hence, we don't have to take any stress over modifying the data directly. We can directly pass in the page store instance to the
pageListItem
where it belongs or to thepageDetails
. There a page can perform operations for a single page rather than altering a massive store of data, which was the drawback of the previous approach. Along with that the hierarchy runs in the top to down manner, where the project store deals with the page store instances but the page store has nothing to do with the project store directly.A glimpse of the performance improvement over the previous version
Switching Between Pages
Screen.Recording.2024-01-18.at.5.35.06.PM.mov
Performing Actions Over Pages
Screen.Recording.2024-01-18.at.5.04.28.PM.mov
Performing Operations inside page details
Screen.Recording.2024-01-18.at.5.06.14.PM.mov
Writing and Quickly Switching Pages
Screen.Recording.2024-01-18.at.5.08.29.PM.mov
Internal Changelog what has changed inside the codebase and how the implementation works
Introduction of a new collection store
ProjectPageStore
and changes in the Collection Data StructureIn the previous implementation of the store in the page store, everything was handled inside the page store, wether it is creating a new page or altering a page, which has been modified now, we have a superior page which handles the mapping between project and pages as discussed above and also it manages the collection operations such as creating or deleting the page. It can be accessed using the
useProjectPages
hookChanges in the
PageStore
As discussed above the previous implementation of the page store used to focus upon the all the collection and the individual page operations, but now with the introduction of the
ProjectPageStore
, PageStores are only responsible for the operations performed for a specific page, like locking.Usage of optimistic updates throughout the actions in both the
PageStore
andProjectPageStore
To make things better and faster we came up with the approach of optimistic updates, where we tend to update the UI first then we make the API Call, where we are optimistic that 95% of the time, the API Call won't fail and updating the UI first will give a faster experience, on a contrary if the api call fails, we tend to rollback the changes we made in the catch block. You can understand that with the below code.
Page Store's Reaction based update process of title and description
Both updating title and description of the page, needs debouncing and performing updated and also making api calls with an external debounce function would make the process more hefty and complicated. Hence, for only updating the title and the description delayed reactions make the process super smooth and all the super-fast saving of the description is the result of the reactions itself. Along with that there is an additional cleanup function that triggers the disposer functions for each of the reactions when the page is unmounted. Below is a small example for the reactions
Changes in the
usePage
hook and introduction ofuseProjectPages
hookPreviously the
usePage
hook used to provide the central store, now the use page hook has been changed to provide the dedicated store for a page, with the help of thepageId
provided in the parameters. TheuseProjectPages
hook will return the central project page store, where we are mapping the projects to the pages, it can be used where we have to perform collection operations over the pages, like adding or deleting the page.Changes in the Issue Suggestion and Isolated logic of issue embeds in
useIssueEmbed
hookFor just another improvement, previously we had all the code for the issue embeds lying around the page, which I decided to separate and created a new hook called
useIssueEmbed
for that, which is responsible for providing the issues with the desired format.