Skip to content
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.

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

Migrate @storybook/instrumenter to strict TS #22370

Merged
merged 10 commits into from
May 8, 2023
2 changes: 1 addition & 1 deletion code/lib/instrumenter/src/instrumenter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class HTMLElement {
}
}

delete global.location;
delete (global as Partial<typeof globalThis>).location;
// @ts-expect-error (global scope type conflicts)
global.location = { reload: jest.fn() };
// @ts-expect-error (global scope type conflicts)
kyletsang marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
10 changes: 7 additions & 3 deletions code/lib/instrumenter/src/instrumenter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ const isObject = (o: unknown) => Object.prototype.toString.call(o) === '[object
const isModule = (o: unknown) => Object.prototype.toString.call(o) === '[object Module]';
const isInstrumentable = (o: unknown) => {
if (!isObject(o) && !isModule(o)) return false;
if (o.constructor === undefined) return true;
const proto = o.constructor.prototype;
if ((o as object).constructor === undefined) return true;
const proto = (o as object).constructor.prototype;
kyletsang marked this conversation as resolved.
Show resolved Hide resolved
if (!isObject(proto)) return false;
if (Object.prototype.hasOwnProperty.call(proto, 'isPrototypeOf') === false) return false;
return true;
Expand Down Expand Up @@ -112,6 +112,10 @@ export class Instrumenter {
isPlaying?: boolean;
isDebugging?: boolean;
}) => {
if (!storyId) {
return;
Copy link

Choose a reason for hiding this comment

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

Presumably this should return a boolean, never void.

}

const state = this.getState(storyId);
kyletsang marked this conversation as resolved.
Show resolved Hide resolved
this.setState(storyId, {
...getInitialState(),
Expand Down Expand Up @@ -576,7 +580,7 @@ export class Instrumenter {
}

const hasPrevious = logItems.some((item) =>
[CallStates.DONE, CallStates.ERROR].includes(item.status)
([CallStates.DONE, CallStates.ERROR] as (CallStates | undefined)[]).includes(item.status)
kyletsang marked this conversation as resolved.
Show resolved Hide resolved
);
const controlStates: ControlStates = {
start: hasPrevious,
Expand Down
11 changes: 9 additions & 2 deletions code/lib/instrumenter/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,14 @@ export interface SyncPayload {
}

export interface State {
renderPhase: 'loading' | 'rendering' | 'playing' | 'played' | 'completed' | 'aborted' | 'errored';
renderPhase?:
| 'loading'
| 'rendering'
| 'playing'
| 'played'
| 'completed'
| 'aborted'
| 'errored';
isDebugging: boolean;
isPlaying: boolean;
isLocked: boolean;
Expand All @@ -73,7 +80,7 @@ export interface State {
ancestors: Call['id'][];
playUntil?: Call['id'];
resolvers: Record<Call['id'], Function>;
syncTimeout: ReturnType<typeof setTimeout>;
syncTimeout?: ReturnType<typeof setTimeout>;
forwardedException?: Error;
}

Expand Down
2 changes: 1 addition & 1 deletion code/lib/instrumenter/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
"extends": "../../tsconfig.json",
"include": ["src/**/*"],
"compilerOptions": {
"strict": false
"strict": true
}
}