consume readBlob in dds #4856
consume readBlob in dds #4856
Conversation
| @@ -242,10 +242,11 @@ export class SharedCell<T extends Serializable = any> extends SharedObject<IShar | |||
| * {@inheritDoc @fluidframework/shared-object-base#SharedObject.loadCore} | |||
| */ | |||
| protected async loadCore(storage: IChannelStorageService): Promise<void> { | |||
| const rawContent = await storage.read(snapshotFileName); | |||
| const blob = await storage.readBlob(snapshotFileName); | |||
There was a problem hiding this comment.
I really dislike existing duplication of code.
In my view, readBlob, bufferToString & JSON.parse should be refactored to be replaced by calling readAndParse().
If it was done in the first place (before you :)), it would make your work trivial - just change 2 lines of code and all of repo is using different flow.
Same arguments works for future - maybe we will decide that for all or some scenarios we want to encode/decode everything in utf-16 - it would be so much simpler if such logic exists in a single place.
There was a problem hiding this comment.
so I should change readAndParse to consume readBlob and then replace code like above with it?
There was a problem hiding this comment.
Yes, that would be preference. It can go in either order - you could first change all this code to use readAndParse, and then change it in later PR.
There was a problem hiding this comment.
You may get this PR in as is (given it's ready), and following up here with separate PR.
■ @fluidframework/base-host: No change
⯅ @fluid-example/bundle-size-tests: +481 Bytes
Baseline commit: 7b3b6c1 |
|
|
||
| // for back compat. will be replaced with readBlob after version 0.35 | ||
| const res = await this.storage.read(id); | ||
| return stringToBuffer(res, "base64"); |
There was a problem hiding this comment.
if we use this.storage.readBlob here now, the back compat tests will fail, because it will call the old readBlob which does not read from cache like read. So I'm only keeping this as read here. If I can debug and find the solution before version 0.35 is ready, I will change it back. Otherwise I will wait for 0.35.
There was a problem hiding this comment.
Sounds good - please open an issue not to forget
Step 4 of #4464