Skip to content

Commit bcbd8b6

Browse files
summeroffclaude
andcommitted
address PR review feedback on generated JS surface
CI: make ci/check-js-generated.sh fail-fast (set -euo pipefail) so a failing git invocation can no longer leave `dirty` empty and pass silently; drop the no-op `set +x`. module.ts typing accuracy (verified against obs-studio-client/source): - IProperty.next()/previous() and IProperties.first()/last()/get() return undefined at end-of-list / empty / not-found, so type them `IProperty | undefined`; fix the stale next() JSDoc and get() "null". - Move sendMessage() from ISource to IInput — only Input registers it natively (input.cpp); Filter/Scene/Transition do not. - Hoist load() to the ISource base (registered on Input/Filter/Scene/ Transition) and drop the per-interface duplicates; IFilter now gets it. - IModuleFactory.modules() returns primitive strings -> string[]. - IModule.initialize() returns a boolean natively, not void. Remove stray trailing `;` after enum/interface blocks so the generated module.js no longer emits empty statements. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 65c9163 commit bcbd8b6

4 files changed

Lines changed: 33 additions & 54 deletions

File tree

ci/check-js-generated.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22
# Verify that the committed tsc outputs under js/ match what regenerating
33
# from js/module.ts would produce. CI runs `yarn build:javascript` first,
44
# so any change under js/ at this point is a stale generated file.
5+
set -euo pipefail
6+
57
dirty=$(git status --porcelain -- js/)
68

7-
set +x
89
if [[ $dirty ]]; then
910
echo "================================================="
1011
echo "Generated JS files are stale. Run locally:"

js/module.d.ts

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -426,17 +426,17 @@ export interface IProperty {
426426
readonly visible: boolean;
427427
readonly type: EPropertyType;
428428
readonly value: any;
429-
next(): IProperty;
430-
previous(): IProperty;
429+
next(): IProperty | undefined;
430+
previous(): IProperty | undefined;
431431
is_first(): boolean;
432432
is_last(): boolean;
433433
modified(): boolean;
434434
}
435435
export interface IProperties {
436-
first(): IProperty;
437-
last(): IProperty;
436+
first(): IProperty | undefined;
437+
last(): IProperty | undefined;
438438
count(): number;
439-
get(name: string): IProperty;
439+
get(name: string): IProperty | undefined;
440440
}
441441
export interface IFactoryTypes {
442442
types(): string[];
@@ -531,7 +531,7 @@ export interface IInput extends ISource {
531531
restart(): void;
532532
stop(): void;
533533
getMediaState(): number;
534-
load(): void;
534+
sendMessage(message: ISettings): void;
535535
}
536536
export interface ISceneFactory {
537537
create(name: string): IScene;
@@ -548,7 +548,6 @@ export interface IScene extends ISource {
548548
getItemAtIdx(idx: number): ISceneItem;
549549
getItems(): ISceneItem[];
550550
getItemsInRange(fromIndex: number, toIndex: number): ISceneItem[];
551-
load(): void;
552551
sendMouseClick(eventData: IMouseEvent, type: EMouseButtonType, mouseUp: boolean, clickCount: number): void;
553552
sendMouseMove(eventData: IMouseEvent, mouseLeave: boolean): void;
554553
sendMouseWheel(eventData: IMouseEvent, x_delta: number, y_delta: number): void;
@@ -595,7 +594,6 @@ export interface ITransition extends ISource {
595594
clear(): void;
596595
set(input: ISource): void;
597596
start(ms: number, input: ISource): void;
598-
load(): void;
599597
sendMouseClick(eventData: IMouseEvent, type: EMouseButtonType, mouseUp: boolean, clickCount: number): void;
600598
sendMouseMove(eventData: IMouseEvent, mouseLeave: boolean): void;
601599
sendMouseWheel(eventData: IMouseEvent, x_delta: number, y_delta: number): void;
@@ -611,7 +609,7 @@ export interface IConfigurable {
611609
export interface ISource extends IConfigurable, IReleasable {
612610
remove(): void;
613611
save(): void;
614-
sendMessage(message: ISettings): void;
612+
load(): void;
615613
readonly status: number;
616614
readonly type: ESourceType;
617615
readonly id: string;
@@ -686,10 +684,10 @@ export interface IAudioFactory {
686684
}
687685
export interface IModuleFactory {
688686
open(binPath: string, dataPath: string): IModule;
689-
modules(): String[];
687+
modules(): string[];
690688
}
691689
export interface IModule {
692-
initialize(): void;
690+
initialize(): boolean;
693691
readonly fileName: string;
694692
readonly name: string;
695693
readonly author: string;

js/module.js

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -383,16 +383,12 @@ var EInteractionFlags;
383383
EInteractionFlags[EInteractionFlags["IsLeft"] = 1024] = "IsLeft";
384384
EInteractionFlags[EInteractionFlags["IsRight"] = 2048] = "IsRight";
385385
})(EInteractionFlags || (exports.EInteractionFlags = EInteractionFlags = {}));
386-
;
387386
var EMouseButtonType;
388387
(function (EMouseButtonType) {
389388
EMouseButtonType[EMouseButtonType["Left"] = 0] = "Left";
390389
EMouseButtonType[EMouseButtonType["Middle"] = 1] = "Middle";
391390
EMouseButtonType[EMouseButtonType["Right"] = 2] = "Right";
392391
})(EMouseButtonType || (exports.EMouseButtonType = EMouseButtonType = {}));
393-
;
394-
;
395-
;
396392
function addItems(scene, sceneItems) {
397393
const items = [];
398394
if (Array.isArray(sceneItems)) {
@@ -502,7 +498,6 @@ var VCamOutputType;
502498
VCamOutputType[VCamOutputType["ProgramView"] = 3] = "ProgramView";
503499
VCamOutputType[VCamOutputType["PreviewOutput"] = 4] = "PreviewOutput";
504500
})(VCamOutputType || (exports.VCamOutputType = VCamOutputType = {}));
505-
;
506501
const appleBinaryFolder = hasDeveloperApp
507502
? path.join(__dirname, 'OSN.app', 'distribute', 'obs-studio-node', 'bin')
508503
: path.join(__dirname, 'bin');

js/module.ts

Lines changed: 22 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -697,19 +697,16 @@ export interface IProperty {
697697
readonly value: any;
698698

699699
/**
700-
* Uses the current object to obtain the next
701-
* property in the properties list.
702-
*
703-
* @returns If it's successful, returns true.
704-
* Otherwise or if end of the list, returns false.
700+
* Uses the current object to obtain the next property in the list.
701+
* @returns The next property, or undefined at the end of the list.
705702
*/
706-
next(): IProperty;
703+
next(): IProperty | undefined;
707704

708705
/**
709706
* Uses the current object to obtain the previous property in the list.
710-
* Returns undefined when the current property is the first.
707+
* @returns The previous property, or undefined when the current property is the first.
711708
*/
712-
previous(): IProperty;
709+
previous(): IProperty | undefined;
713710

714711
/** True when this property is the first in the list. */
715712
is_first(): boolean;
@@ -728,20 +725,20 @@ export interface IProperty {
728725
*/
729726
export interface IProperties {
730727

731-
/** Obtains the first property in the list. */
732-
first(): IProperty;
728+
/** Obtains the first property in the list, or undefined when the list is empty. */
729+
first(): IProperty | undefined;
733730

734-
/** Obtains the last property in the list. */
735-
last(): IProperty;
731+
/** Obtains the last property in the list, or undefined when the list is empty. */
732+
last(): IProperty | undefined;
736733

737734
count(): number;
738735

739736
/**
740737
* Obtains property matching name.
741738
* @param name The name of the property to fetch.
742-
* @returns - The property instance or null if not found
739+
* @returns - The property instance, or undefined if not found
743740
*/
744-
get(name: string): IProperty;
741+
get(name: string): IProperty | undefined;
745742
}
746743

747744
export interface IFactoryTypes {
@@ -819,27 +816,27 @@ export enum EInteractionFlags {
819816
IsKeyPad = 1 << 9,
820817
IsLeft = 1 << 10,
821818
IsRight = 1 << 11
822-
};
819+
}
823820

824821
export enum EMouseButtonType {
825822
Left,
826823
Middle,
827824
Right
828-
};
825+
}
829826

830827
export interface IMouseEvent {
831828
modifiers: EInteractionFlags;
832829
x: number;
833830
y: number;
834-
};
831+
}
835832

836833
export interface IKeyEvent {
837834
modifiers: EInteractionFlags;
838835
text: string;
839836
nativeModifiers: number;
840837
nativeScancode: number;
841838
nativeVkey: number;
842-
};
839+
}
843840

844841
export interface ISceneItemInfo {
845842
name: string,
@@ -976,9 +973,9 @@ export interface IInput extends ISource {
976973
getMediaState(): number;
977974

978975
/**
979-
* Re-trigger the source's load step (re-reads serialized state on the server).
976+
* Forward a serializable message to the underlying source plugin.
980977
*/
981-
load(): void;
978+
sendMessage(message: ISettings): void;
982979
}
983980

984981
export interface ISceneFactory {
@@ -1074,11 +1071,6 @@ export interface IScene extends ISource {
10741071
*/
10751072
getItemsInRange(fromIndex: number, toIndex: number): ISceneItem[];
10761073

1077-
/**
1078-
* Re-trigger the scene's load step on the server.
1079-
*/
1080-
load(): void;
1081-
10821074
sendMouseClick(eventData: IMouseEvent, type: EMouseButtonType, mouseUp: boolean, clickCount: number): void;
10831075
sendMouseMove(eventData: IMouseEvent, mouseLeave: boolean): void;
10841076
sendMouseWheel(eventData: IMouseEvent, x_delta: number, y_delta: number): void;
@@ -1241,11 +1233,6 @@ export interface ITransition extends ISource {
12411233
*/
12421234
start(ms: number, input: ISource): void;
12431235

1244-
/**
1245-
* Re-trigger the transition's load step on the server.
1246-
*/
1247-
load(): void;
1248-
12491236
sendMouseClick(eventData: IMouseEvent, type: EMouseButtonType, mouseUp: boolean, clickCount: number): void;
12501237
sendMouseMove(eventData: IMouseEvent, mouseLeave: boolean): void;
12511238
sendMouseWheel(eventData: IMouseEvent, x_delta: number, y_delta: number): void;
@@ -1295,11 +1282,9 @@ export interface ISource extends IConfigurable, IReleasable {
12951282
save(): void;
12961283

12971284
/**
1298-
* Forward a serializable message to the underlying source plugin.
1299-
* Note: only registered on input sources on the native side; calling on
1300-
* a filter, scene, or transition will throw at runtime.
1285+
* Re-trigger the source's load step on the server.
13011286
*/
1302-
sendMessage(message: ISettings): void;
1287+
load(): void;
13031288

13041289
/**
13051290
* The validity of the source
@@ -1498,11 +1483,11 @@ export interface IAudioFactory {
14981483

14991484
export interface IModuleFactory {
15001485
open(binPath: string, dataPath: string): IModule;
1501-
modules(): String[];
1486+
modules(): string[];
15021487
}
15031488

15041489
export interface IModule {
1505-
initialize(): void;
1490+
initialize(): boolean;
15061491
readonly fileName: string;
15071492
readonly name: string;
15081493
readonly author: string;
@@ -1927,7 +1912,7 @@ export enum VCamOutputType {
19271912
SourceOutput,
19281913
ProgramView,
19291914
PreviewOutput,
1930-
};
1915+
}
19311916

19321917
// Initialization and other stuff which needs local data.
19331918
const appleBinaryFolder = hasDeveloperApp

0 commit comments

Comments
 (0)