Skip to content

Refactor serializedStateManager fetchSnapshot#20191

Merged
dannimad merged 2 commits intomicrosoft:mainfrom
dannimad:snapshot-free
Mar 18, 2024
Merged

Refactor serializedStateManager fetchSnapshot#20191
dannimad merged 2 commits intomicrosoft:mainfrom
dannimad:snapshot-free

Conversation

@dannimad
Copy link
Copy Markdown
Contributor

Some refactoring aroung serializedStateManager.fetchSnapshot including:

  • making a clear difference between the two flows involving whether or not we're loading using a pendinglocalstate or not instead of having both flows awkwardly merged.
  • making 2 free functions to get the snapshot and version from storage depending on which api we're using (getSnapshot or getSnapshotTree)
  • fusion "Fluid.Container.UseLoadingGroupIdForSnapshotFetch" with supportGetSnapshotApi since they both serve the same purpose of just choosing between getSnapshot or getSnapshotFree.

I considered writing some unit tests for the new free functions but they don't do anything special but handling undefined cases.

@github-actions github-actions Bot added area: loader Loader related issues base: main PRs targeted against main branch labels Mar 18, 2024
}
return { snapshotTree, version };
} else {
this.snapshot = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit, destructor the object

Suggested change
this.snapshot = {
const { baseSnapshot, snapshotBlobs} =this.pendingLocalState;
this.snapshot = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could also make this.snapshot match pending local state in terms of property names.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea. I'll do that in another PR since it we will need to change those names in several places

@msfluid-bot
Copy link
Copy Markdown
Collaborator

msfluid-bot commented Mar 18, 2024

Baseline CI build failed, cannot generate bundle analysis at this time


Baseline commit: 418468c

Generated by 🚫 dangerJS against a5ae5e6

Comment thread packages/loader/container-loader/src/serializedStateManager.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: loader Loader related issues base: main PRs targeted against main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants