Skip to content

fix: Add guard to normalizeKeyName() #98

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.

Sign up for GitHub

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

Merged
KevinVandy merged 5 commits into from
Mar 31, 2026
Merged

fix: Add guard to normalizeKeyName() #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5acb4fb
Add guard to normalizeKeyName()
ryck Mar 31, 2026
f04822e
Add tests
ryck Mar 31, 2026
b65e589
Fix typo
ryck Mar 31, 2026
e727777
add changeset
KevinVandy Mar 31, 2026
9be240b
regen docs
KevinVandy Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/early-candles-warn.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tanstack/hotkeys': patch
---

fix: normalizeKeyName null check
2 changes: 1 addition & 1 deletion docs/reference/variables/KEY_DISPLAY_SYMBOLS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ title: KEY_DISPLAY_SYMBOLS
const KEY_DISPLAY_SYMBOLS: object;
```

Defined in: [constants.ts:589](https://github.com/TanStack/hotkeys/blob/main/packages/hotkeys/src/constants.ts#L589)
Defined in: [constants.ts:592](https://github.com/TanStack/hotkeys/blob/main/packages/hotkeys/src/constants.ts#L592)

Special key symbols for display formatting.

Expand Down
2 changes: 1 addition & 1 deletion docs/reference/variables/LINUX_MODIFIER_LABELS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ title: LINUX_MODIFIER_LABELS
const LINUX_MODIFIER_LABELS: Record<CanonicalModifier | "Mod", string>;
```

Defined in: [constants.ts:556](https://github.com/TanStack/hotkeys/blob/main/packages/hotkeys/src/constants.ts#L556)
Defined in: [constants.ts:559](https://github.com/TanStack/hotkeys/blob/main/packages/hotkeys/src/constants.ts#L559)
2 changes: 1 addition & 1 deletion docs/reference/variables/MAC_MODIFIER_LABELS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ title: MAC_MODIFIER_LABELS
const MAC_MODIFIER_LABELS: Record<CanonicalModifier | "Mod", string>;
```

Defined in: [constants.ts:522](https://github.com/TanStack/hotkeys/blob/main/packages/hotkeys/src/constants.ts#L522)
Defined in: [constants.ts:525](https://github.com/TanStack/hotkeys/blob/main/packages/hotkeys/src/constants.ts#L525)

Modifier key labels for macOS display.

Expand Down
2 changes: 1 addition & 1 deletion docs/reference/variables/MAC_MODIFIER_SYMBOLS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ title: MAC_MODIFIER_SYMBOLS
const MAC_MODIFIER_SYMBOLS: Record<CanonicalModifier | "Mod", string>;
```

Defined in: [constants.ts:499](https://github.com/TanStack/hotkeys/blob/main/packages/hotkeys/src/constants.ts#L499)
Defined in: [constants.ts:502](https://github.com/TanStack/hotkeys/blob/main/packages/hotkeys/src/constants.ts#L502)

Modifier key symbols for macOS display.

Expand Down
2 changes: 1 addition & 1 deletion docs/reference/variables/PUNCTUATION_KEY_DISPLAY_LABELS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ title: PUNCTUATION_KEY_DISPLAY_LABELS
const PUNCTUATION_KEY_DISPLAY_LABELS: object;
```

Defined in: [constants.ts:562](https://github.com/TanStack/hotkeys/blob/main/packages/hotkeys/src/constants.ts#L562)
Defined in: [constants.ts:565](https://github.com/TanStack/hotkeys/blob/main/packages/hotkeys/src/constants.ts#L565)

## Type Declaration

Expand Down
2 changes: 1 addition & 1 deletion docs/reference/variables/WINDOWS_MODIFIER_LABELS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ title: WINDOWS_MODIFIER_LABELS
const WINDOWS_MODIFIER_LABELS: Record<CanonicalModifier | "Mod", string>;
```

Defined in: [constants.ts:545](https://github.com/TanStack/hotkeys/blob/main/packages/hotkeys/src/constants.ts#L545)
Defined in: [constants.ts:548](https://github.com/TanStack/hotkeys/blob/main/packages/hotkeys/src/constants.ts#L548)

Modifier key labels for Windows/Linux display.

Expand Down
3 changes: 3 additions & 0 deletions packages/hotkeys/src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,9 @@ export function isSingleLetterKey(key: string): boolean {
* ```
*/
export function normalizeKeyName(key: string): string {
// key can be undefined in rare cases
// (browser extensions synthesizing key events, accessibility tools, certain OS/browser combinations).
if (!key) return ''
Comment on lines +458 to +460
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect direct call sites that normalize event.key.
rg -n --type=ts -C3 '\bnormalizeKeyName\s*\(\s*event\.key\s*\)' packages/hotkeys/src

# Inspect likely downstream consumers where '' may be treated as a valid key.
rg -n --type=ts -C3 '\bheldKeysSet\.add\(|\bheldCodesMap\.set\(|\bisModifierKey\s*\(\s*normalizeKeyName\s*\(\s*event\.key\s*\)\s*\)' packages/hotkeys/src

Repository: TanStack/hotkeys

Length of output: 5912

🏁 Script executed:

# First, let's see the full implementation of normalizeKeyName
sed -n '450,470p' packages/hotkeys/src/constants.ts

# Then check the isModifierKey function
rg -n --type=ts 'export function isModifierKey|export const isModifierKey' packages/hotkeys/src -A 10

# Also check what normalizeKeyName does with empty/falsy values
sed -n '440,465p' packages/hotkeys/src/constants.ts

Repository: TanStack/hotkeys

Length of output: 2317

🏁 Script executed:

# Check key-state-tracker to see if there's any guard before adding to Set
sed -n '110,130p' packages/hotkeys/src/key-state-tracker.ts

# Check if normalizeKeyName can actually return empty string, and check parse.ts usage
rg -n --type=ts 'const normalizedKey = normalizeKeyName' packages/hotkeys/src -A 5

Repository: TanStack/hotkeys

Length of output: 1669

🏁 Script executed:

# Check match.ts for how it handles empty eventKey
sed -n '63,75p' packages/hotkeys/src/match.ts

# Check hotkey-manager.ts for releasedKey usage
sed -n '635,650p' packages/hotkeys/src/hotkey-manager.ts

Repository: TanStack/hotkeys

Length of output: 1037

Line 460: Avoid returning '' for invalid keys; use null and require callers to short-circuit.

The guard prevents a crash but lets invalid events (with undefined event.key) flow as empty-string sentinels into downstream stateful paths. For example, in key-state-tracker.ts:113–116, the empty string gets added directly to #heldKeysSet and #heldCodesMap with no validation. Returning null instead makes the invalid state explicit and forces callers to handle it.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/hotkeys/src/constants.ts` around lines 458 - 460, The guard in
constants.ts currently returns an empty string for an invalid/undefined key
which lets invalid keys flow downstream; change the sentinel to return null
instead of '' so callers must short-circuit and handle the absence of a key.
Update the function that checks `key` (the if (!key) return ...) to return null,
and then update any callers that consume its result—particularly code paths that
add to `#heldKeysSet` and `#heldCodesMap` in key-state-tracker.ts—to explicitly
check for null and skip processing. Ensure all uses of the function treat null
as “invalid key” (not a valid key string) so no empty-string values are inserted
into heldKeys/heldCodes structures.

Copy link
Copy Markdown
Contributor Author

@ryck ryck Mar 31, 2026

Choose a reason for hiding this comment

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

I used '' so the return type stays string, but happy to be corrected...

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

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

🧩 Analysis chain

Good question — let me look at the actual downstream call sites to give you a concrete recommendation.

🏁 Script executed:

#!/bin/bash
# Check the full context around key-state-tracker.ts where normalizeKeyName result is used
rg -n --type=ts 'normalizeKeyName' packages/hotkeys/src -A 5 -B 3

# Also check if there's any existing empty-string guard at call sites
rg -n --type=ts 'normalizedKey' packages/hotkeys/src -A 3 -B 3

Length of output: 15632

@ryck Keeping string is the right call here — changing to string | null would cascade through 7+ call sites across the codebase. The '' sentinel is actually fine for most of them:

Call site Effect with ''
match.ts:66 eventKey.length === 1 is false → safely skipped
hotkey-manager.ts:638 '' won't match any registered key → harmless
parse.ts:267 Returns ParsedHotkey with key: '' → no hotkey will match it
sequence-manager.ts:442, recorder-chord.ts:17 isModifierKey('') → false, minor but non-critical

The one concrete problem is key-state-tracker.ts lines 113–116, where no guard exists and '' would be stored directly into #heldKeysSet and #heldCodesMap:

const key = normalizeKeyName(event.key)
if (!this.#heldKeysSet.has(key)) {
  this.#heldKeysSet.add(key)        // ← '' inserted here
  this.#heldCodesMap.set(key, event.code)
}

The fix is simple — a single guard at that call site, keeping your string return type intact:

// key-state-tracker.ts, keydown listener
this.#keydownListener = (event: KeyboardEvent) => {
  const key = normalizeKeyName(event.key)
  if (!key) return  // ← guard against undefined event.key
  if (!this.#heldKeysSet.has(key)) {
    this.#heldKeysSet.add(key)
    this.#heldCodesMap.set(key, event.code)
    this.#syncState()
  }
}

So: your normalizeKeyName guard is correct, and '' is the right sentinel value. The remaining gap is just this one guard in key-state-tracker.ts.

// Check aliases first
if (key in KEY_ALIASES) {
return KEY_ALIASES[key]!
Expand Down
7 changes: 7 additions & 0 deletions packages/hotkeys/tests/parse.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,13 @@ describe('isModifierKey', () => {
})
})

describe('normalizeKeyName', () => {
it('returns empty string for falsy input (undefined coerced, empty string)', () => {
expect(normalizeKeyName('')).toBe('')
expect(normalizeKeyName(undefined as unknown as string)).toBe('')
})
})

describe('rawHotkeyToParsedHotkey', () => {
it('should convert minimal RawHotkey (key only)', () => {
const result = rawHotkeyToParsedHotkey({ key: 'Escape' })
Expand Down
Loading