refactor(container-runtime): Enable any-related eslint rules and fix violations
#23570
Conversation
github-actions
bot
added
area: runtime
There was a problem hiding this comment.
Copilot reviewed 22 out of 37 changed files in this pull request and generated no comments.
Files not reviewed (15)
- packages/runtime/container-runtime/src/test/containerRuntime.spec.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/deltaManagerProxies.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/test/channelCollection.spec.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/test/dataStoreContext.spec.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/opLifecycle/opGroupingManager.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/opLifecycle/opCompressor.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/test/batching.spec.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/dataStore.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/summary/summarizer.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/test/createChildDataStoreSync.spec.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/summary/summaryManager.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/opLifecycle/outbox.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/containerRuntime.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/channelCollection.ts: Evaluated as low risk
- packages/runtime/container-runtime/src/opLifecycle/opSplitter.ts: Evaluated as low risk
github-actions
bot
added
area: build
| @@ -203,6 +203,7 @@ | |||
| "@fluidframework/test-runtime-utils": "workspace:~", | |||
| "@microsoft/api-extractor": "7.47.8", | |||
| "@types/double-ended-queue": "^2.1.0", | |||
| "@types/lz4js": "^0.2.0", | |||
| @@ -215,6 +216,7 @@ export class OpSplitter { | |||
| // back-compat with 1.x builds | |||
| // This is only required / present for non-compressed, chunked ops | |||
| // For compressed ops, we have op grouping enabled, and type of each op is preserved within compressed content. | |||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment | |||
There was a problem hiding this comment.
Seeing this originalType used in two places but requiring the cast makes me very suspicious of the typing here 🤔 .
There was a problem hiding this comment.
Yeah. This was odd enough that I erred on the side of leaving it alone.
| // TODO: better typing here | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment |
There was a problem hiding this comment.
This seems to work: const errorObj = Error as unknown as { stackTraceLimit: number };
There was a problem hiding this comment.
Fair. But I wasn't sure if there was a real type we were working around here. I think I would rather leave this alone (with the TODO) so someone with more familiarity could update with the right type.
There was a problem hiding this comment.
As far as I could tell, the issue is just that stackTraceLimit is non-standard according to the docs, so it's not present on the Error type. But fair enough, can be left alone for now.
packages/runtime/container-runtime/src/opLifecycle/opDecompressor.ts
Outdated
Show resolved
Hide resolved
packages/runtime/container-runtime/src/test/pendingStateManager.spec.ts
Outdated
Show resolved
Hide resolved
Generally speaking, I'm good with that. Eslint-disable comments, maybe less so 😜 |
Josmithr
changed the title
no-explicit-any eslint rule and fix violationsany-related eslint rules and fix violations
Enables
any-related rules from the recommended eslint config and fixes violations. Specifically,@typescript-eslint/no-explicit-any@typescript-eslint/no-unsafe-assignment@typescript-eslint/no-unsafe-call@typescript-eslint/no-unsafe-member-access@typescript-eslint/no-unsafe-returnThis PR is one piece of a multi-PR process to migrate the
container-runtimepackage to our recommended eslint config.AB#3027