-
Notifications
You must be signed in to change notification settings - Fork 16
refactor: replace string event IDs with EventID #200
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,3 +1,4 @@ | ||||||
| import { EventID } from 'packages/room/dist'; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 🧩 Analysis chainFix EventID import path; prefer type‑only import. Use the same source as the rest of the repo and avoid pulling runtime code for a type. Apply: -import { EventID } from 'packages/room/dist';
+import type { EventID } from '@hs/room';The EventID[] parameter changes on Lines 37–38 look correct and align with the project‑wide migration. Also applies to: 37-38 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== show target file (first 200 lines) ==="
if [ -f packages/core/src/events/m.room.join_rules.ts ]; then
sed -n '1,240p' packages/core/src/events/m.room.join_rules.ts || true
else
echo "MISSING: packages/core/src/events/m.room.join_rules.ts"
fi
echo
echo "=== show lines 1-120 with numbers for quick reference ==="
if [ -f packages/core/src/events/m.room.join_rules.ts ]; then
nl -ba -w3 -s': ' packages/core/src/events/m.room.join_rules.ts | sed -n '1,120p' || true
fi
echo
echo "=== search repo for EventID symbol usages (context) ==="
if command -v rg >/dev/null 2>&1; then
rg -n --hidden -S '\bEventID\b' || true
else
grep -RIn --line-number '\bEventID\b' . || true
fi
echo
echo "=== search for imports that pull from packages/room/dist ==="
if command -v rg >/dev/null 2>&1; then
rg -n --hidden -S "packages/room/dist" || true
else
grep -RIn --line-number "packages/room/dist" . || true
fi
echo
echo "=== search for @hs/room occurrences ==="
if command -v rg >/dev/null 2>&1; then
rg -n --hidden -S "@hs/room" || true
else
grep -RIn --line-number "@hs/room" . || true
fi
echo
echo "=== packages/room package.json (if present) ==="
if [ -f packages/room/package.json ]; then
sed -n '1,240p' packages/room/package.json || true
else
echo "MISSING: packages/room/package.json"
fi
echo
echo "=== list top files under packages/room ==="
if [ -d packages/room ]; then
find packages/room -maxdepth 3 -type f | sed -n '1,200p' || true
else
echo "MISSING: packages/room directory"
fi
echo
echo "=== show root package.json and tsconfig.json (first 200 lines) ==="
if [ -f package.json ]; then
sed -n '1,240p' package.json || true
else
echo "MISSING: package.json"
fi
echo
if [ -f tsconfig.json ]; then
sed -n '1,240p' tsconfig.json || true
else
echo "MISSING: tsconfig.json"
fiLength of output: 29169 Fix EventID import — use a type-only import from @hs/room Apply: -import { EventID } from 'packages/room/dist';
+import type { EventID } from '@hs/room';auth_events / prev_events (lines 37–38) are already correctly typed as EventID[]. Also update other occurrences that import from 'packages/room/dist' (e.g. packages/core/src/utils/generateId.ts, packages/core/src/events/*.ts, packages/federation-sdk/src/services/missing-event.service.ts). 📝 Committable suggestion
Suggested change
|
||||||
| import { type EventBase, createEventBase } from './eventBase'; | ||||||
| import { createEventWithId } from './utils/createSignedEvent'; | ||||||
|
|
||||||
|
|
@@ -33,8 +34,8 @@ export const roomJoinRulesEvent = ({ | |||||
| }: { | ||||||
| roomId: string; | ||||||
| sender: string; | ||||||
| auth_events: string[]; | ||||||
| prev_events: string[]; | ||||||
| auth_events: EventID[]; | ||||||
| prev_events: EventID[]; | ||||||
| depth: number; | ||||||
| ts?: number; | ||||||
| }) => { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,3 +1,4 @@ | ||||||
| import { EventID } from 'packages/room/dist'; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix import path and make it type‑only. Use the package alias consistently and avoid a runtime import. -import { EventID } from 'packages/room/dist';
+import type { EventID } from '@hs/room';📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| import { createEventBase } from './eventBase'; | ||||||
| import type { Membership, RoomMemberEvent } from './isRoomMemberEvent'; | ||||||
| import { createEventWithId } from './utils/createSignedEvent'; | ||||||
|
|
@@ -18,12 +19,12 @@ declare module './eventBase' { | |||||
| } | ||||||
|
|
||||||
| export type AuthEvents = { | ||||||
| 'm.room.create': string; | ||||||
| 'm.room.power_levels'?: string; | ||||||
| 'm.room.join_rules'?: string; | ||||||
| 'm.room.history_visibility'?: string; | ||||||
| 'm.room.create': EventID; | ||||||
| 'm.room.power_levels'?: EventID; | ||||||
| 'm.room.join_rules'?: EventID; | ||||||
| 'm.room.history_visibility'?: EventID; | ||||||
| } & { | ||||||
| [K in `m.room.member:${string}`]?: string; | ||||||
| [K in `m.room.member:${string}`]?: EventID; | ||||||
| }; | ||||||
|
|
||||||
| const isTruthy = <T>( | ||||||
|
|
@@ -50,7 +51,7 @@ export const roomMemberEvent = ({ | |||||
| sender: string; | ||||||
| state_key: string; | ||||||
| auth_events: AuthEvents; | ||||||
| prev_events: string[]; | ||||||
| prev_events: EventID[]; | ||||||
| depth: number; | ||||||
| unsigned?: RoomMemberEvent['unsigned']; | ||||||
| content?: Record<string, any>; | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,3 +1,4 @@ | ||||||
| import { EventID } from 'packages/room/dist'; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainUse a type‑only import for EventID to avoid runtime dependency on 'packages/room/dist'. Prevents accidental runtime import under verbatimModuleSyntax/ESM. -import { EventID } from 'packages/room/dist';
+import type { EventID } from 'packages/room/dist';Please confirm tsconfig has either 🏁 Script executed: #!/bin/bash
fd -a 'tsconfig*.json' | xargs -I{} jq -r '.compilerOptions.importsNotUsedAsValues?, .compilerOptions.verbatimModuleSyntax?' {} 2>/dev/nullLength of output: 64 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf "PWD: %s\n\n" "$(pwd)"
echo "=== tsconfig files ==="
fd -HI -a 'tsconfig*.json' 2>/dev/null || true
for f in $(fd -HI -a 'tsconfig*.json' 2>/dev/null || true); do
echo -e "\n--- $f ---"
# print the two properties if present, otherwise indicate MISSING
jq -r 'if .compilerOptions then
"importsNotUsedAsValues: " + (.compilerOptions.importsNotUsedAsValues // "MISSING") + "\nverbatimModuleSyntax: " + ( (.compilerOptions.verbatimModuleSyntax // "MISSING") | tostring )
else
"no compilerOptions"
end' "$f" 2>/dev/null || { echo "jq failed; printing first 200 chars:"; sed -n '1,40p' "$f"; }
done
echo -e "\n=== packages/core/src/events/m.room.name.ts ==="
if [ -f packages/core/src/events/m.room.name.ts ]; then
sed -n '1,200p' packages/core/src/events/m.room.name.ts
else
echo "file not found at that exact path; searching for matching files..."
fd -HI 'm.room.name.ts' || true
fi
echo -e "\n=== Search for EventID import patterns ==="
rg -n --hidden --no-ignore --glob '!node_modules' "import\s+type\s*\{[^}]*\bEventID\b[^}]*\}" -S || true
rg -n --hidden --no-ignore --glob '!node_modules' "import\s*\{[^}]*\bEventID\b[^}]*\}" -S || true
echo -e "\n=== Search for imports from packages/room/dist ==="
rg -n --hidden --no-ignore --glob '!node_modules' "from\s+['\"]packages/room/dist['\"]" -S || true
echo -e "\n=== package.json typescript version (if present) ==="
if [ -f package.json ]; then
jq -r '.devDependencies.typescript // .dependencies.typescript // "missing"' package.json 2>/dev/null || true
else
echo "package.json not found at repo root"
fiLength of output: 7223 Use a type-only import for EventID (avoid runtime dependency on 'packages/room/dist') packages/core/src/events/m.room.name.ts currently uses a value import and no repo tsconfig sets importsNotUsedAsValues or verbatimModuleSyntax, so this will be emitted at runtime. Replace with a type-only import or set importsNotUsedAsValues: "remove" in the relevant tsconfig. -import { EventID } from 'packages/room/dist';
+import type { EventID } from 'packages/room/dist';📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| import { type EventBase, createEventBase } from './eventBase'; | ||||||
| import { createEventWithId } from './utils/createSignedEvent'; | ||||||
|
|
||||||
|
|
@@ -16,9 +17,9 @@ declare module './eventBase' { | |||||
| } | ||||||
|
|
||||||
| export type RoomNameAuthEvents = { | ||||||
| 'm.room.create': string; | ||||||
| 'm.room.power_levels': string; | ||||||
| 'm.room.member': string; | ||||||
| 'm.room.create': EventID; | ||||||
| 'm.room.power_levels': EventID; | ||||||
| 'm.room.member': EventID; | ||||||
| }; | ||||||
|
|
||||||
| export const isRoomNameEvent = (event: EventBase): event is RoomNameEvent => { | ||||||
|
|
@@ -52,7 +53,7 @@ export const roomNameEvent = ({ | |||||
| roomId: string; | ||||||
| sender: string; | ||||||
| auth_events: RoomNameAuthEvents; | ||||||
| prev_events: string[]; | ||||||
| prev_events: EventID[]; | ||||||
| depth: number; | ||||||
| content: { | ||||||
| name: string; | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,15 @@ | ||
| import crypto from 'node:crypto'; | ||
| import { EventID } from 'packages/room/dist'; | ||
| import { toUnpaddedBase64 } from './binaryData'; | ||
| import { pruneEventDict } from './pruneEventDict'; | ||
| import { encodeCanonicalJson } from './signJson'; | ||
|
|
||
| export function generateId<T extends object>(content: T): string { | ||
| export function generateId<T extends object>(content: T): EventID { | ||
| // remove the fields that are not part of the hash | ||
| const { unsigned, signatures, ...toHash } = pruneEventDict(content as any); | ||
|
|
||
| return `\$${toUnpaddedBase64( | ||
| crypto.createHash('sha256').update(encodeCanonicalJson(toHash)).digest(), | ||
| { urlSafe: true }, | ||
| )}`; | ||
| )}` as EventID; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong import path for EventID; use package alias.
Using 'packages/room/dist' will break outside the monorepo build. Import from '@hs/room' as a type.
Apply:
📝 Committable suggestion
🤖 Prompt for AI Agents