refactor(shared-object-base): Move to recommended eslint base config #23233
Conversation
github-actions
bot
added
area: dds
| @@ -52,7 +48,7 @@ export class RemoteFluidObjectHandle extends FluidHandleBase<FluidObject> { | |||
| }; | |||
| this.objectP = this.routeContext.resolveHandle(request).then<FluidObject>((response) => { | |||
| if (response.mimeType === "fluid/object") { | |||
| const fluidObject: FluidObject = response.value; | |||
| const fluidObject: FluidObject = response.value as FluidObject; | |||
There was a problem hiding this comment.
Any validation we can do here?
There was a problem hiding this comment.
Not familiar with the space, but I think the existing check for mimeType is enough?
packages/dds/shared-object-base/src/test/attachingBindingAndConnecting.spec.ts
Outdated
Show resolved
Hide resolved
packages/dds/shared-object-base/src/test/attachingBindingAndConnecting.spec.ts
Outdated
Show resolved
Hide resolved
| @@ -234,16 +248,18 @@ describe("FluidSerializer", () => { | |||
| ); | |||
|
|
|||
| // Add some handles to intermediate objects. | |||
| input.h = handle; | |||
| input.o1.h = handle; | |||
| input.h = handle; // eslint-disable-line @typescript-eslint/no-unsafe-member-access | |||
There was a problem hiding this comment.
Can we not use eslint-disable-next-line for these (for consistency)?
There was a problem hiding this comment.
We can. I personally find the resulting code harder to parse than this, but don't feel strongly one way or the other. I tend to prefer "this-line" comments and use next-line only when the line is already too long and reading the disable would require scrolling.
There was a problem hiding this comment.
I don't feel that strongly, it's just not what we normally do. Feel free to close without action 🙂
There was a problem hiding this comment.
I didn't even know there was a "this-line"!! I'm with @alexvy86 - I like that much better than "next-line" as long as it doesn't make the line way too long.
Co-authored-by: Joshua Smithrud <[email protected]>
Co-authored-by: Joshua Smithrud <[email protected]>
| @@ -33,10 +33,10 @@ export interface ISharedObjectKind<TSharedObject> { | |||
| } | |||
|
|
|||
| // @alpha | |||
| export function makeHandlesSerializable(value: any, serializer: IFluidSerializer, bind: IFluidHandle): any; | |||
| export function makeHandlesSerializable(value: unknown, serializer: IFluidSerializer, bind: IFluidHandle): any; | |||
There was a problem hiding this comment.
Updating an input type on free functions shouldn't be breaking.
| let didAttach = 0; | ||
| let attachCalled = false; | ||
| const { overrides, sharedObject } = createTestSharedObject({ | ||
| runtime: { | ||
| ...(runtimeEvents as any), | ||
| ...(runtimeEvents as unknown as Overridable<IFluidDataStoreRuntime>), |
There was a problem hiding this comment.
Using the same pattern found already in other places in this file. Trying to use ...runtimeEvents on its own results in mismatched typing issues along the lines of the one below, which I don't quite understand or know how to fix:
<several more entries...>
Type 'TypedEventTransform<TypedEventEmitter<IFluidDataStoreRuntimeEvents>, IFluidDataStoreRuntimeEvents>' is not assignable to type 'TransformedEvent<IFluidDataStoreRuntime, "disconnected", []>'.
Types of parameters 'event' and 'event' are incompatible.
Type '"disconnected"' is not assignable to type '"newListener" | "removeListener"'.ts(2322)
|
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output |
Description
Moves from the minimal-deprecated eslint config to the recommended one in the shared-object-base package.
Reviewer Guidance
The review process is outlined on this wiki page.
AB#3018