improvement(client): misc cleanup #23959
Conversation
- Cleanup `IContainerRuntimeOptionsInternal` use versus `IContainerRuntimeOptions` (prep for future expansion) - Comment corrections, enhancement, and cleanup - Type-only imports for containerRuntime.ts externals
jason-ha
requested review from
Copilot and
markfields
and removed request for
Copilot
github-actions
bot
added
area: runtime
| @@ -952,7 +957,7 @@ export class ContainerRuntime | |||
| context: IContainerContext; | |||
| registryEntries: NamedFluidDataStoreRegistryEntries; | |||
| existing: boolean; | |||
| runtimeOptions?: IContainerRuntimeOptions; // May also include options from IContainerRuntimeOptionsInternal | |||
There was a problem hiding this comment.
Any changes here should also happen for loadContainerRuntime free function (see LoadContainerRuntimeParams). We probably should update this to use that params type. Or just get rid of it, unless we really need an internal version that returns concrete ContainerRuntime
There was a problem hiding this comment.
Ok, obviously we can't make these changes on LoadContainerRuntimeParams because that's not @internal. This is fine for now.
| @@ -1552,7 +1557,7 @@ export class ContainerRuntime | |||
| electedSummarizerData: ISerializedElection | undefined, | |||
| chunks: [string, string[]][], | |||
| dataStoreAliasMap: [string, string][], | |||
| baseRuntimeOptions: Readonly<Required<IContainerRuntimeOptions>>, | |||
| runtimeOptions: Readonly<Required<IContainerRuntimeOptionsInternal>>, | |||
| // Can the `content` argument type be IEnvelope? | ||
| // verifyNotClosed is called in FluidDataStoreContext, which is *the* expected caller. |
There was a problem hiding this comment.
Is this a note to self? Not sure it should be checked in in its current state
There was a problem hiding this comment.
not self notes really. I changed the first to be a Future: comment and the latter is just interesting to know.
I do have work in progress for the new addressing pattern. That made me think about the call flow; so, I captured the verifyNoClosed note.
- Cleanup `IContainerRuntimeOptionsInternal` use versus `IContainerRuntimeOptions` (prep for future expansion) - Comment corrections, enhancement, and cleanup - Type-only imports for containerRuntime.ts externals
IContainerRuntimeOptionsInternaluse versusIContainerRuntimeOptions(prep for future expansion)