Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Revert "Enable noUncheckedIndexedAccess for routerlicious (#21487)"" #23063

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RishhiB
Copy link
Contributor

@RishhiB RishhiB commented Nov 12, 2024

Reverts #22382

@github-actions github-actions bot added area: driver Driver related issues base: main PRs targeted against main branch labels Nov 12, 2024
Copy link
Collaborator

@msfluid-bot msfluid-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Coverage Summary

↓ packages.drivers.routerlicious-urlResolver.src:
Line Coverage Change: -0.83%    Branch Coverage Change: No change
Metric NameBaseline coveragePR coverageCoverage Diff
Branch Coverage 84.00% 84.00% → No change
Line Coverage 92.97% 92.14% ↓ -0.83%
↑ packages.drivers.routerlicious-driver.src:
Line Coverage Change: 0.02%    Branch Coverage Change: No change
Metric NameBaseline coveragePR coverageCoverage Diff
Branch Coverage 76.69% 76.69% → No change
Line Coverage 54.63% 54.65% ↑ 0.02%

Baseline commit: c7730cc
Baseline build: 306006
Happy Coding!!

Code coverage comparison check passed!!

@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: +297 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 464.33 KB 464.36 KB +35 Bytes
azureClient.js 563.18 KB 563.28 KB +101 Bytes
connectionState.js 724 Bytes 724 Bytes No change
containerRuntime.js 262.48 KB 262.5 KB +14 Bytes
fluidFramework.js 426.84 KB 426.85 KB +14 Bytes
loader.js 134.18 KB 134.19 KB +14 Bytes
map.js 42.71 KB 42.71 KB +7 Bytes
matrix.js 148.36 KB 148.36 KB +7 Bytes
odspClient.js 528.97 KB 529.02 KB +49 Bytes
odspDriver.js 97.88 KB 97.9 KB +21 Bytes
odspPrefetchSnapshot.js 42.81 KB 42.83 KB +14 Bytes
sharedString.js 164.26 KB 164.27 KB +7 Bytes
sharedTree.js 417.3 KB 417.3 KB +7 Bytes
Total Size 3.37 MB 3.37 MB +297 Bytes

Baseline commit: c7730cc

Generated by 🚫 dangerJS against 9cd96d5

@RishhiB RishhiB marked this pull request as draft November 12, 2024 18:36

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 12 changed files in this pull request and generated no suggestions.

Files not reviewed (7)
  • packages/drivers/routerlicious-driver/tsconfig.json: Language not supported
  • packages/drivers/routerlicious-urlResolver/tsconfig.json: Language not supported
  • packages/drivers/routerlicious-driver/src/deltaStorageService.ts: Evaluated as low risk
  • packages/drivers/routerlicious-driver/src/urlUtils.ts: Evaluated as low risk
  • packages/drivers/routerlicious-driver/src/createNewUtils.ts: Evaluated as low risk
  • packages/drivers/routerlicious-driver/src/documentServiceFactory.ts: Evaluated as low risk
  • packages/drivers/routerlicious-driver/src/documentService.ts: Evaluated as low risk
Comments skipped due to low confidence (6)

packages/drivers/routerlicious-urlResolver/src/urlResolver.ts:67

  • The non-null assertion on 'path[2]' should be reviewed to ensure it is necessary and correctly used.
tenantId = path[2]!;

packages/drivers/routerlicious-urlResolver/src/urlResolver.ts:70

  • The non-null assertion on 'path[3]' should be reviewed to ensure it is necessary and correctly used.
documentId = path[3]!;

packages/drivers/routerlicious-urlResolver/src/urlResolver.ts:75

  • The non-null assertion on 'path[2]' should be reviewed to ensure it is necessary and correctly used.
documentId = path[2]!;

packages/drivers/routerlicious-driver/src/summaryTreeUploadManager.ts:150

  • The non-null assertion on 'previousSnapshot.trees[key]' should be justified or removed. The TODO comment indicates uncertainty, which needs to be addressed.
return this.getIdFromPathCore(handleType, path.slice(1), previousSnapshot.trees[key]!);

packages/drivers/routerlicious-driver/src/r11sSnapshotParser.ts:40

  • The non-null assertion on lookup[entryPathDir] assumes that entryPathDir will always be a valid key. Clarify this assumption or add a check to ensure lookup[entryPathDir] is not undefined before using it.
const node = lookup[entryPathDir]!;

packages/drivers/routerlicious-driver/src/r11sSnapshotParser.ts:80

  • The non-null assertion on flatSnapshot.trees[0] assumes that flatSnapshot.trees will always have at least one element. Add a check to ensure flatSnapshot.trees is not empty before accessing the first element.
const flatSnapshotTree = flatSnapshot.trees[0]!;

Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: driver Driver related issues base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants