Skip to content

Commit f23dd45

Browse files
committed
fix(core): warn once per action, not once per key-set
Keying the warn-once memo on the unknown keys alone reported the FIRST action carrying `targt` and stayed silent about every other one — sending the author to fix a button that was only the first symptom. Keyed by action name too; the memo is bounded by the number of authored actions either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rvv6qysks2dRLaGfpTdgEy
1 parent 08b83e0 commit f23dd45

2 files changed

Lines changed: 20 additions & 6 deletions

File tree

packages/core/src/actions/__tests__/actionKeys.pin.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,20 @@ describe('unknown-key warning', () => {
147147
expect(warn.mock.calls[0][0]).toContain('rename the key to `target`');
148148
});
149149

150-
it('warns once per key, not once per click', () => {
150+
it('warns once per action, not once per click', () => {
151151
for (let i = 0; i < 5; i++) warnOnUnknownActionKeys({ name: 'save', type: 'script', targt: 'x' });
152152
expect(warn).toHaveBeenCalledTimes(1);
153153
});
154154

155+
it('still names a SECOND action carrying the same typo', () => {
156+
// Keying the warn-once memo on the keys alone would report `save` and stay
157+
// silent about `remove` — sending the author to fix the first symptom only.
158+
warnOnUnknownActionKeys({ name: 'save', type: 'script', targt: 'x' });
159+
warnOnUnknownActionKeys({ name: 'remove', type: 'script', targt: 'y' });
160+
expect(warn).toHaveBeenCalledTimes(2);
161+
expect(warn.mock.calls[1][0]).toContain('"remove"');
162+
});
163+
155164
it('is silent in production', () => {
156165
const prev = process.env.NODE_ENV;
157166
process.env.NODE_ENV = 'production';

packages/core/src/actions/actionKeys.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,9 +202,14 @@ export function classifyActionKeys(action: object | null | undefined): {
202202
return { unknown, retired };
203203
}
204204

205-
// Warn once per key, not once per execution: an unrecognized key usually sits in
206-
// metadata driving a button that gets clicked repeatedly, and a warning that
207-
// floods the console is a warning that gets muted.
205+
// Warn once per (action, problem), not once per execution: an unrecognized key
206+
// usually sits in metadata driving a button that gets clicked repeatedly, and a
207+
// warning that floods the console is a warning that gets muted.
208+
//
209+
// Keyed by action name as well as by the keys, deliberately. Keying on the keys
210+
// alone would report the FIRST action carrying `targt` and stay silent about
211+
// every other one — sending the author to fix a button that was only the first
212+
// symptom. The memo is bounded by the number of authored actions either way.
208213
const warned = new Set<string>();
209214

210215
/** Reset the warn-once memo. Exported for tests. */
@@ -230,14 +235,14 @@ export function warnOnUnknownActionKeys(action: object | null | undefined, where
230235
const name = (action as { name?: unknown; type?: unknown })?.name ?? (action as { type?: unknown })?.type ?? '(unnamed)';
231236

232237
for (const key of retired) {
233-
const memo = `retired:${key}`;
238+
const memo = `retired:${String(name)}:${key}`;
234239
if (warned.has(memo)) continue;
235240
warned.add(memo);
236241
console.warn(`[${where}] action "${String(name)}" carries the retired key \`${key}\`. ${RETIRED_ACTION_KEYS[key]}`);
237242
}
238243

239244
if (unknown.length === 0) return;
240-
const memo = `unknown:${unknown.slice().sort().join(',')}`;
245+
const memo = `unknown:${String(name)}:${unknown.slice().sort().join(',')}`;
241246
if (warned.has(memo)) return;
242247
warned.add(memo);
243248
console.warn(

0 commit comments

Comments
 (0)