Description
In Action<T extends JsonObject>, both getSettings and setSettings introduce their own generic type parameter U extends JsonObject = T.
Because U is not constrained by T (only defaults to it), callers can explicitly supply any U extends JsonObject and bypass the Action<T> type entirely. As a result, Action<T> does not actually enforce the shape of the settings it is supposed to represent.
In addition, setSettings will infer U from the argument passed, so even without explicitly providing a generic parameter, it still accepts settings objects unrelated to T.
Examples
type MySettings = {a: 42};
class Demo extends SingletonAction<MySettings> {
override async onWillAppear(ev: WillAppearEvent<MySettings>): Promise<void> {
await ev.action.setSettings({b: ""});
(await ev.action.getSettings()).a.toFixed(); // crash
}
}
Impact
- The
T parameter on Action<T> is effectively advisory rather than enforced.
- Callers can persist settings with a completely different object shape than
T.
getSettings allows consumers to “assert” any return type, even if it does not match the runtime payload.
- This weakens type safety and makes refactors and schema changes more error-prone.
Suggested fix
setSettings(): Promise<T> and setSettings(settings: T): Promise<void>
I supply this as an issue rather than a pull request because it might break the current API for some devs.
Description
In
Action<T extends JsonObject>, bothgetSettingsandsetSettingsintroduce their own generic type parameterU extends JsonObject = T.Because
Uis not constrained byT(only defaults to it), callers can explicitly supply anyU extends JsonObjectand bypass theAction<T>type entirely. As a result,Action<T>does not actually enforce the shape of the settings it is supposed to represent.In addition,
setSettingswill inferUfrom the argument passed, so even without explicitly providing a generic parameter, it still accepts settings objects unrelated toT.Examples
Impact
Tparameter onAction<T>is effectively advisory rather than enforced.T.getSettingsallows consumers to “assert” any return type, even if it does not match the runtime payload.Suggested fix
setSettings(): Promise<T>andsetSettings(settings: T): Promise<void>I supply this as an issue rather than a pull request because it might break the current API for some devs.