Skip to content

Commit e005cc1

Browse files
Apollon77claude
Apollon77
and
claude
authored
fix(node): tighten StateSelector to typed behavior keys (#3683)
* fix(node): tighten StateSelector to typed behavior keys Resolves the TODO from 2d9d35f. The previous comment claimed the SupportedBehaviors index signature blocked tightening, but concrete EndpointType.For<T> already preserves the literal MapOf<> shape with narrow keys. Use a `string extends K` discriminator inside StateSelector so concrete typed endpoints get BehaviorSelection<BehaviorAt<T,K>> (typed attribute keys, autocomplete, rejection of unknown keys), and unbound `T extends EndpointType` falls back to RawBehaviorSelection. Adds a @ts-expect-error type-level test asserting unknown attribute keys are rejected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(node): assert StateSelector fallback for unbound generic endpoints Adds a type-level equality assertion that the `string extends K` fallback branch of `StateSelector<T>` resolves to `RawBehaviorSelection` for unbound `T extends EndpointType`, plus a runtime-shape assertion that arbitrary string-keyed attribute lists remain assignable. Closes the regression gap noted by Copilot review on PR #3683 — without this, a refactor that drops or reorders the fallback branch could silently break generic callers that relied on raw string selections. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(node): exercise real EndpointType()/SupportedBehaviors() factory path Adds a type-level regression test that builds an endpoint via the actual `EndpointType(...)` + `SupportedBehaviors(...)` + `MapOf<>` pipeline and asserts: (a) `keyof behaviors` stays narrow to the literal id rather than widening to `string`, and (b) `StateSelector<T>` accepts known State keys and rejects unknown ones. Closes the gap noted by Copilot review on PR #3683 — the previous tests only used a hand-built intersection (`EndpointType & { behaviors: { fake: ... } }`) that would still pass even if `EndpointType.For<T>` or `MapOf<T>` regressed for real, factory- constructed endpoints. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5aca7b8 commit e005cc1

2 files changed

Lines changed: 54 additions & 5 deletions

File tree

‎packages/node/src/endpoint/Endpoint.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,10 +1347,12 @@ export type BehaviorSelection<B extends Behavior.Type> = true | readonly (keyof
13471347

13481348
export type RawBehaviorSelection = true | readonly string[];
13491349

1350-
// TODO: use BehaviorSelection<BehaviorAt<T, K>> — blocked by SupportedBehaviors index sig (Record<string,...>
1351-
// makes keyof=string → BehaviorAt→Behavior.Type → keyof {}=never). Fix: remove index sig. getStateOf() is ok.
13521350
export type StateSelector<T extends EndpointType> = {
1353-
[K in keyof T["behaviors"]]?: RawBehaviorSelection;
1351+
[K in keyof T["behaviors"]]?: string extends K
1352+
? RawBehaviorSelection
1353+
: T["behaviors"][K] extends Behavior.Type
1354+
? BehaviorSelection<T["behaviors"][K]>
1355+
: RawBehaviorSelection;
13541356
};
13551357

13561358
export type StateSliceOf<T extends EndpointType, S> = S extends undefined

‎packages/node/test/endpoint/EndpointGetTypesTest.ts‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7-
import type { Behavior } from "#behavior/Behavior.js";
7+
import { Behavior } from "#behavior/Behavior.js";
88
import type {
99
BehaviorAt,
1010
BehaviorOf,
@@ -13,7 +13,8 @@ import type {
1313
StateSelector,
1414
StateSliceOf,
1515
} from "#endpoint/Endpoint.js";
16-
import type { EndpointType } from "#endpoint/type/EndpointType.js";
16+
import { SupportedBehaviors } from "#endpoint/properties/SupportedBehaviors.js";
17+
import { EndpointType } from "#endpoint/type/EndpointType.js";
1718
import type { Immutable } from "@matter/general";
1819

1920
type FakeState = { value: number; label: string };
@@ -62,6 +63,52 @@ describe("Endpoint get type helpers", () => {
6263
const _selectorEmpty: StateSelector<TestEndpoint> = {};
6364
void _selectorEmpty;
6465

66+
// StateSelector rejects attribute keys that are not in the behavior's State for typed endpoints.
67+
// @ts-expect-error - "missing" is not a key of FakeState
68+
const _selectorBadKey: StateSelector<TestEndpoint> = { fake: ["missing"] as const };
69+
void _selectorBadKey;
70+
71+
// Fallback branch: unbound `T extends EndpointType` widens `keyof T["behaviors"]` to `string`
72+
// via the SupportedBehaviors index signature, so values must accept RawBehaviorSelection
73+
// (arbitrary string-keyed attribute lists) for backward compatibility with generic code.
74+
type _GenericValue = NonNullable<StateSelector<EndpointType>[string]>;
75+
type _AssertEqual<A, B> = (<T>() => T extends A ? 1 : 0) extends <T>() => T extends B ? 1 : 0 ? true : false;
76+
const _genericValueIsRaw: _AssertEqual<_GenericValue, RawBehaviorSelection> = true;
77+
void _genericValueIsRaw;
78+
const _genericSelector: StateSelector<EndpointType> = { someBehavior: ["arbitraryAttr"] as const };
79+
void _genericSelector;
80+
81+
// Real factory path: exercise EndpointType(...) + SupportedBehaviors(...) + MapOf<>, not a
82+
// hand-built intersection. Catches regressions in EndpointType.For<T> / MapOf<T> that would
83+
// widen `keyof T["behaviors"]` back to `string` and silently drop the per-behavior tightening.
84+
class FactoryTestBehavior extends Behavior {
85+
static override readonly id = "factoryTest";
86+
static override readonly State = class FactoryTestState {
87+
value = 0;
88+
label = "";
89+
};
90+
}
91+
const FactoryTestEndpoint = EndpointType({
92+
name: "FactoryTest",
93+
deviceType: 0xfff1,
94+
deviceRevision: 1,
95+
behaviors: SupportedBehaviors(FactoryTestBehavior),
96+
});
97+
type _FactoryTestType = typeof FactoryTestEndpoint;
98+
99+
// Behavior keys must remain narrow (literal "factoryTest"), not widened to `string`.
100+
type _FactoryKeys = keyof _FactoryTestType["behaviors"];
101+
const _factoryKeysNarrow: string extends _FactoryKeys ? false : true = true;
102+
void _factoryKeysNarrow;
103+
104+
// Typed attribute key accepted on factory-built endpoint
105+
const _factoryGood: StateSelector<_FactoryTestType> = { factoryTest: ["value"] as const };
106+
void _factoryGood;
107+
108+
// @ts-expect-error - "missing" is not a key of FactoryTestBehavior State
109+
const _factoryBad: StateSelector<_FactoryTestType> = { factoryTest: ["missing"] as const };
110+
void _factoryBad;
111+
65112
// StateSliceOf with { fake: true } produces a slice with full fake state
66113
type _TrueSlice = StateSliceOf<TestEndpoint, { fake: true }>;
67114
const _checkTrueSlice: _TrueSlice = {} as { fake: Immutable<Behavior.StateOf<FakeBehaviorType>> };

0 commit comments

Comments
 (0)