eslint(container-loader): Prefix container-loader before enabling no-unchecked-record-access #23423
Conversation
…unchecked-record-access
github-actions
bot
added
area: loader
There was a problem hiding this comment.
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| baseSnapshot.trees.default.trees.root.unreferenced, | ||
| baseSnapshot.trees.default?.trees.root?.unreferenced, |
There was a problem hiding this comment.
@RishhiB, what do this and other ? additions have to do with no-unchecked-record-access rule?
There was a problem hiding this comment.
I believe its because baseSnapshot.trees is of type [path: string]: ISnapshotTree, so default or root record access may not exist
| const attributesId = pendingState.baseSnapshot.trees[".protocol"].blobs.attributes; | ||
| const attributes = pendingState.snapshotBlobs[attributesId]; | ||
| const attributesId: string | undefined = | ||
| pendingState.baseSnapshot.trees[".protocol"]?.blobs.attributes; | ||
| const attributes: string | undefined = pendingState.snapshotBlobs[attributesId]; | ||
| return JSON.parse(attributes) as IDocumentAttributes; |
There was a problem hiding this comment.
Looks like this pattern isn't actually working for us.
JSON.parse does not accept undefined. It only accepts string.
If you hover on attributes on line 160 in VS Code, it will say the type is string. TypeScript appears to ignore the explicit type when it thinks it can successfully narrow the type to something more specific.
It doesn't look like we can use this pattern to be safe. (It might be okay if the linter followed the uses of the variables to double check typing. I would guess that is pretty hard.)
Prefix container-loader before enabling no-unchecked-record-access
AB#26471