fix: replace account-data as-any TODOs with explanatory comments (#502)#508
Conversation
Sentry Preview Error TriageNo Sentry errors found for this PR's preview deployment as of Thu, 02 Jul 2026 14:33:47 GMT. This comment updates automatically after each push. |
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce unhelpful tech-debt TODOs by replacing them with explanatory comments that justify unavoidable TypeScript casts around Matrix SDK typing gaps, tying the rationale back to issue #502.
Changes:
- Replaced vague “remove cast once typing is unified” TODOs with structured explanations in developer-tools account data flows.
- Updated the RoomCallButton TODO with an explanatory comment (intended to document a cast around SDK typings).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/app/features/settings/developer-tools/DevelopTools.tsx |
Replaces TODOs with detailed rationale for as never casts when setting/getting arbitrary account-data types. |
src/app/features/settings/developer-tools/AccountData.tsx |
Replaces TODOs with explanatory comments around account-data key listing/refreshing logic. |
src/app/features/room/RoomCallButton.tsx |
Replaces a TODO with an explanatory comment intended to justify event typing casts for an MSC event. |
Sentry Snapshot Testing
|
Code Coverage OverviewLanguages: JavaScript JavaScript / code-coverage/vitestThe overall coverage remains at 30%, unchanged from the branch. Updated |
Apply prettier formatting (double quotes, trailing commas, parameter wrapping)
Apply prettier formatting (double quotes, trailing commas)
|
🚧 Skipped: PR exceeds review size limit. Please split into smaller PRs and re-run. |
| } | ||
| > | ||
| <PageNavHeader outlined={!hasBanner} size="600"> | ||
| {hideText ? ( | ||
| <Box alignItems="Center" grow="Yes" justifyContent="Center"> | ||
| <Avatar size={hideText ? undefined : '200'} radii="400" onClick={handleOpenMenu}> | ||
| <Avatar | ||
| size={hideText ? undefined : "200"} |
|
OpenTofu plan for production Plan: 3 to add, 0 to change, 3 to destroy.OpenTofu used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement
OpenTofu will perform the following actions:
# cloudflare_worker_version.site must be replaced
-/+ resource "cloudflare_worker_version" "site" {
!~ annotations = {
+ workers_message = (known after apply)
+ workers_tag = (known after apply)
!~ workers_triggered_by = "create_version_api" -> (known after apply)
} -> (known after apply)
!~ assets = { # forces replacement
!~ asset_manifest_sha256 = "cfb29faf79c62cc3ab25e96e1f8c626997964acf0104c8d1c40e1161c4b80710" -> "31cd6d61e277370fbda342f933c7764947bfc78f22b649f9d88ff34fd8b2c3bf" # forces replacement
!~ directory = "/home/runner/work/Charm/Charm/dist/client" -> "/github/workspace/dist/client"
# (1 unchanged attribute hidden)
}
+ compatibility_flags = (known after apply)
!~ created_on = "2026-06-23T18:49:50Z" -> (known after apply)
!~ id = "************************************" -> (known after apply)
+ limits = (known after apply)
+ main_script_base64 = (known after apply)
!~ modules = [
- {
- content_file = "/home/runner/work/Charm/Charm/dist/charm/index.js" -> null
- content_sha256 = "ba9b71550741749400afaa0bbd6f38b92f9607890c4f2b3265e68241823277b9" -> null
- content_type = "application/javascript+module" -> null
- name = "index.js" -> null
},
+ {
+ content_file = "/github/workspace/dist/charm/index.js"
+ content_sha256 = "ba9b71550741749400afaa0bbd6f38b92f9607890c4f2b3265e68241823277b9"
+ content_type = "application/javascript+module"
+ name = "index.js"
},
]
!~ number = 1 -> (known after apply)
!~ source = "terraform" -> (known after apply)
!~ startup_time_ms = 0 -> (known after apply)
# (5 unchanged attributes hidden)
}
# cloudflare_workers_custom_domain.site must be replaced
-/+ resource "cloudflare_workers_custom_domain" "site" {
- environment = "production" -> null # forces replacement
!~ id = "****************************************" -> (known after apply)
!~ zone_name = "cloudhub.social" -> (known after apply)
# (4 unchanged attributes hidden)
}
# cloudflare_workers_deployment.site must be replaced
-/+ resource "cloudflare_workers_deployment" "site" {
!~ annotations = { # forces replacement
!~ workers_message = "integration" -> "fix: replace account-data as-any TODOs with explanatory comments (#502)"
!~ workers_triggered_by = "deployment" -> (known after apply)
}
+ author_email = (known after apply)
!~ created_on = "2026-06-23T18:49:50Z" -> (known after apply)
!~ id = "************************************" -> (known after apply)
!~ source = "terraform" -> (known after apply)
!~ versions = [ # forces replacement
!~ {
!~ version_id = "************************************" -> (known after apply)
# (1 unchanged attribute hidden)
},
]
# (3 unchanged attributes hidden)
}
Plan: 3 to add, 0 to change, 3 to destroy.📝 Plan generated in Cloudflare Infra #230 |
a5fa3c0 to
952f057
Compare
Deploying with
|
| Status | Preview URL | Commit | Alias | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! | https://pr-508-charm.justin-tech.workers.dev | 222ea74 | pr-508 |
Thu, 02 Jul 2026 14:35:46 GMT |
Root Cause
matrix-js-sdk'ssetAccountData/getAccountDatahave typed overloads only for known event types. For arbitrary/developer-tool event types, TypeScript requires a cast. The four TODO comments ("remove once account data typing is unified") referenced a future unification that never happened, providing no guidance to future readers.Fix
Replaced all four vague TODO comments with structured explanatory comments documenting:
as nevercast is required given matrix-js-sdk's type signaturesas anycasts) #502)The
as nevercasts themselves remain (they are unavoidable given the current SDK API) but are no longer unexplained tech debt.Spec
.claude/specs/issue-502.mdCloses #502