From cf36279adea44f3ec178a0350287b1222d50aae1 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Fri, 10 Jun 2022 15:41:04 +0200 Subject: [PATCH 01/19] Don't intercept calls inside callbacks, and bubble exceptions inside callbacks up to the parent rather than forwarding it to the next interceptable call --- lib/instrumenter/src/instrumenter.test.ts | 66 ++++++++++++++++++++--- lib/instrumenter/src/instrumenter.ts | 59 +++++++++++++------- 2 files changed, 100 insertions(+), 25 deletions(-) diff --git a/lib/instrumenter/src/instrumenter.test.ts b/lib/instrumenter/src/instrumenter.test.ts index 5f6064f5cbd..748cdcd9126 100644 --- a/lib/instrumenter/src/instrumenter.test.ts +++ b/lib/instrumenter/src/instrumenter.test.ts @@ -39,6 +39,8 @@ let instrumenter: Instrumenter; const instrument = >(obj: TObj, options: Options = {}) => instrumenter.instrument(obj, options); +const tick = () => new Promise((resolve) => setTimeout(resolve, 0)); + beforeEach(() => { jest.useRealTimers(); callSpy.mockClear(); @@ -317,6 +319,20 @@ describe('Instrumenter', () => { expect(() => setRenderPhase('played')).toThrow(new Error('Boom!')); }); + it('hoists child exceptions (in callback)', () => { + const { fn1, fn2 } = instrument({ + fn1: (callback?: Function) => callback && callback(), + fn2: () => { + throw new Error('Boom!'); + }, + }); + expect( + fn1(() => { + fn2(); + }) + ).toEqual(new Error('Boom!')); + }); + it("re-throws anything that isn't an error", () => { const { fn } = instrument({ fn: () => { @@ -357,6 +373,41 @@ describe('Instrumenter', () => { describe('with intercept: true', () => { const options = { intercept: true }; + it('only includes intercepted calls in the log', async () => { + const fn = (callback?: Function) => callback && callback(); + const { fn1, fn2 } = instrument({ fn1: fn, fn2: fn }, options); + const { fn3 } = instrument({ fn3: fn }, { intercept: false }); + fn1(); + fn2(); + fn3(); + await tick(); + expect(syncSpy).toHaveBeenCalledWith( + expect.objectContaining({ + logItems: [ + { callId: 'kind--story [0] fn1', status: 'done' }, + { callId: 'kind--story [1] fn2', status: 'done' }, + ], + }) + ); + }); + + it('does not intercept child calls (in callback)', async () => { + const fn = (callback?: Function) => callback && callback(); + const { fn1, fn2 } = instrument({ fn1: fn, fn2: fn }, options); + fn1(() => { + fn2(); + }); + await tick(); + expect(syncSpy).toHaveBeenCalledWith( + expect.objectContaining({ + logItems: [ + { callId: 'kind--story [0] fn1', status: 'done' }, + // Second call is not here because it is not intercepted + ], + }) + ); + }); + it('emits a call event with error data when the function throws', () => { const { fn } = instrument( { @@ -443,12 +494,13 @@ describe('Instrumenter', () => { }); it.skip('starts debugging at the first non-nested interceptable call', () => { - const { fn } = instrument({ fn: jest.fn((...args: any) => args) }, { intercept: true }); - fn(fn(), fn()); // setup the dependencies + const fn = (...args) => args; + const { fn1, fn2, fn3 } = instrument({ fn1: fn, fn2: fn, fn3: fn }, { intercept: true }); + fn3(fn1(), fn2()); // setup the dependencies addons.getChannel().emit(EVENTS.START, { storyId }); - const a = fn('a'); - const b = fn('b'); - const c = fn(a, b); + const a = fn1('a'); + const b = fn2('b'); + const c = fn3(a, b); expect(a).toEqual(['a']); expect(b).toEqual(['b']); expect(c).toEqual(expect.any(Promise)); @@ -476,13 +528,13 @@ describe('Instrumenter', () => { expect(fn).toHaveBeenCalledTimes(0); addons.getChannel().emit(EVENTS.NEXT, { storyId }); - await new Promise((resolve) => setTimeout(resolve, 0)); + await tick(); expect(mockedInstrumentedFn).toHaveBeenCalledTimes(2); expect(fn).toHaveBeenCalledTimes(1); addons.getChannel().emit(EVENTS.END, { storyId }); - await new Promise((resolve) => setTimeout(resolve, 0)); + await tick(); expect(mockedInstrumentedFn).toHaveBeenCalledTimes(3); expect(fn).toHaveBeenCalledTimes(3); diff --git a/lib/instrumenter/src/instrumenter.ts b/lib/instrumenter/src/instrumenter.ts index 9e54071bab6..a167937b0cd 100644 --- a/lib/instrumenter/src/instrumenter.ts +++ b/lib/instrumenter/src/instrumenter.ts @@ -147,6 +147,12 @@ export class Instrumenter { // Rethrow any unhandled forwarded exception so it doesn't go unnoticed. if (forwardedException) throw forwardedException; } + if (newPhase === 'errored') { + this.setState(storyId, { + isLocked: false, + isPlaying: false, + }); + } }); // Trash non-retained state and clear the log when switching stories, but not on initial boot. @@ -331,7 +337,8 @@ export class Instrumenter { this.setState(storyId, { cursor: cursor + 1 }); const id = `${parentId || storyId} [${cursor}] ${method}`; const { path = [], intercept = false, retain = false } = options; - const interceptable = typeof intercept === 'function' ? intercept(method, path) : intercept; + const interceptable = + !parentId && (typeof intercept === 'function' ? intercept(method, path) : intercept); const call: Call = { id, parentId, storyId, cursor, path, method, args, interceptable, retain }; const result = (interceptable ? this.intercept : this.invoke).call(this, fn, call, options); return this.instrument(result, { ...options, mutate: true, path: [{ __callId__: call.id }] }); @@ -418,6 +425,11 @@ export class Instrumenter { throw IGNORED_EXCEPTION; } + // Exceptions inside callbacks should bubble up to the parent call rather than be forwarded. + if (call.parentId) { + throw e; + } + // Non-interceptable calls need their exceptions forwarded to the next interceptable call. // In case no interceptable call picks it up, it'll get rethrown in the "completed" phase. this.setState(call.storyId, { forwardedException: e }); @@ -437,25 +449,36 @@ export class Instrumenter { throw alreadyCompletedException; } - const finalArgs = options.getArgs + // Some libraries override function args through the `getArgs` option. + const actualArgs = options.getArgs ? options.getArgs(call, this.getState(call.storyId)) : call.args; - const result = fn( - // Wrap any callback functions to provide a way to access their "parent" call. - // This is picked up in the `track` function and used for call metadata. - ...finalArgs.map((arg: any) => { - if (typeof arg !== 'function' || Object.keys(arg).length) return arg; - return (...args: any) => { - const { cursor, parentId } = this.getState(call.storyId); - this.setState(call.storyId, { cursor: 0, parentId: call.id }); - const restore = () => this.setState(call.storyId, { cursor, parentId }); - const res = arg(...args); - if (res instanceof Promise) res.then(restore, restore); - else restore(); - return res; - }; - }) - ); + + // Wrap any callback functions to provide a way to access their "parent" call. + // This is picked up in the `track` function and used for call metadata. + const finalArgs = actualArgs.map((arg: any) => { + // We only want to wrap plain functions, not objects. + if (typeof arg !== 'function' || Object.keys(arg).length) return arg; + + // console.log(call.id); + return (...args: any) => { + // Set the cursor and parentId for calls that happen inside the callback. + const { cursor, parentId } = this.getState(call.storyId); + this.setState(call.storyId, { cursor: 0, parentId: call.id }); + const restore = () => this.setState(call.storyId, { cursor, parentId }); + + // Invoke the actual callback function. + const res = arg(...args); + + // Reset cursor and parentId to their original values before we entered the callback. + if (res instanceof Promise) res.then(restore, restore); + else restore(); + + return res; + }; + }); + + const result = fn(...finalArgs); // Track the result so we can trace later uses of it back to the originating call. // Primitive results (undefined, null, boolean, string, number, BigInt) are ignored. From dcd77df12ba637b4a8f6114ddb9328a526b9909e Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Fri, 10 Jun 2022 16:50:51 +0200 Subject: [PATCH 02/19] Show child interactions in log and display errors in the appropriate place --- .../components/Interaction/Interaction.tsx | 3 +- lib/instrumenter/src/instrumenter.test.ts | 11 +++++-- lib/instrumenter/src/instrumenter.ts | 30 ++++++++++--------- lib/instrumenter/src/types.ts | 1 + 4 files changed, 27 insertions(+), 18 deletions(-) diff --git a/addons/interactions/src/components/Interaction/Interaction.tsx b/addons/interactions/src/components/Interaction/Interaction.tsx index d9d6bf27ddb..9f96c9a9585 100644 --- a/addons/interactions/src/components/Interaction/Interaction.tsx +++ b/addons/interactions/src/components/Interaction/Interaction.tsx @@ -27,6 +27,7 @@ const RowContainer = styled('div', { shouldForwardProp: (prop) => !['call'].incl backgroundColor: theme.base === 'dark' ? transparentize(0.93, theme.color.negative) : theme.background.warning, }), + paddingLeft: call.parentId ? 20 : 0, })); const RowLabel = styled('button', { shouldForwardProp: (prop) => !['call'].includes(prop) })< @@ -88,7 +89,7 @@ export const Interaction = ({ {call.status === CallStates.ERROR && - call.exception && + call.exception?.callId === call.id && (call.exception.message.startsWith('expect(') ? ( ) : ( diff --git a/lib/instrumenter/src/instrumenter.test.ts b/lib/instrumenter/src/instrumenter.test.ts index 748cdcd9126..7599bce6b3a 100644 --- a/lib/instrumenter/src/instrumenter.test.ts +++ b/lib/instrumenter/src/instrumenter.test.ts @@ -319,7 +319,7 @@ describe('Instrumenter', () => { expect(() => setRenderPhase('played')).toThrow(new Error('Boom!')); }); - it('hoists child exceptions (in callback)', () => { + it('bubbles child exceptions up to parent (in callback)', () => { const { fn1, fn2 } = instrument({ fn1: (callback?: Function) => callback && callback(), fn2: () => { @@ -391,7 +391,7 @@ describe('Instrumenter', () => { ); }); - it('does not intercept child calls (in callback)', async () => { + it('also includes child calls in the log', async () => { const fn = (callback?: Function) => callback && callback(); const { fn1, fn2 } = instrument({ fn1: fn, fn2: fn }, options); fn1(() => { @@ -402,7 +402,11 @@ describe('Instrumenter', () => { expect.objectContaining({ logItems: [ { callId: 'kind--story [0] fn1', status: 'done' }, - // Second call is not here because it is not intercepted + { + callId: 'kind--story [0] fn1 [0] fn2', + status: 'done', + parentId: 'kind--story [0] fn1', + }, ], }) ); @@ -425,6 +429,7 @@ describe('Instrumenter', () => { name: 'Error', message: 'Boom!', stack: expect.stringContaining('Error: Boom!'), + callId: 'kind--story [0] fn', }, }) ); diff --git a/lib/instrumenter/src/instrumenter.ts b/lib/instrumenter/src/instrumenter.ts index a167937b0cd..06b07b89360 100644 --- a/lib/instrumenter/src/instrumenter.ts +++ b/lib/instrumenter/src/instrumenter.ts @@ -178,7 +178,7 @@ export class Instrumenter { playUntil || shadowCalls .slice(0, firstRowIndex) - .filter((call) => call.interceptable) + .filter((call) => call.interceptable && !call.parentId) .slice(-1)[0]?.id, }; }); @@ -193,7 +193,8 @@ export class Instrumenter { const next = isDebugging ? log.findIndex(({ status }) => status === CallStates.WAITING) : log.length; - start({ storyId, playUntil: log[next - 2]?.callId }); + const playUntil = log[next - 2]?.parentId || log[next - 2]?.callId; + start({ storyId, playUntil }); }; const goto = ({ storyId, callId }: { storyId: string; callId: Call['id'] }) => { @@ -276,7 +277,7 @@ export class Instrumenter { } }); if (call.interceptable && !seen.has(call.id)) { - acc.unshift({ callId: call.id, status: call.status }); + acc.unshift({ callId: call.id, status: call.status, parentId: call.parentId }); seen.add(call.id); } return acc; @@ -337,10 +338,10 @@ export class Instrumenter { this.setState(storyId, { cursor: cursor + 1 }); const id = `${parentId || storyId} [${cursor}] ${method}`; const { path = [], intercept = false, retain = false } = options; - const interceptable = - !parentId && (typeof intercept === 'function' ? intercept(method, path) : intercept); + const interceptable = typeof intercept === 'function' ? intercept(method, path) : intercept; const call: Call = { id, parentId, storyId, cursor, path, method, args, interceptable, retain }; - const result = (interceptable ? this.intercept : this.invoke).call(this, fn, call, options); + const interceptOrInvoke = interceptable && !parentId ? this.intercept : this.invoke; + const result = interceptOrInvoke.call(this, fn, call, options); return this.instrument(result, { ...options, mutate: true, path: [{ __callId__: call.id }] }); } @@ -405,10 +406,10 @@ export class Instrumenter { } }); - const handleException = (e: unknown) => { + const handleException = (e: any) => { if (e instanceof Error) { - const { name, message, stack } = e; - const exception = { name, message, stack }; + const { name, message, stack, callId = call.id } = e as Error & { callId?: Call['id'] }; + const exception = { name, message, stack, callId }; this.update({ ...info, status: CallStates.ERROR, exception }); // Always track errors to their originating call. @@ -419,17 +420,18 @@ export class Instrumenter { ]), })); + // Exceptions inside callbacks should bubble up to the parent call rather than be forwarded. + if (call.parentId) { + Object.defineProperty(e, 'callId', { value: call.id }); + throw e; + } + // We need to throw to break out of the play function, but we don't want to trigger a redbox // so we throw an ignoredException, which is caught and silently ignored by Storybook. if (call.interceptable && e !== alreadyCompletedException) { throw IGNORED_EXCEPTION; } - // Exceptions inside callbacks should bubble up to the parent call rather than be forwarded. - if (call.parentId) { - throw e; - } - // Non-interceptable calls need their exceptions forwarded to the next interceptable call. // In case no interceptable call picks it up, it'll get rethrown in the "completed" phase. this.setState(call.storyId, { forwardedException: e }); diff --git a/lib/instrumenter/src/types.ts b/lib/instrumenter/src/types.ts index 48f17fdec0d..862036ea75a 100644 --- a/lib/instrumenter/src/types.ts +++ b/lib/instrumenter/src/types.ts @@ -47,6 +47,7 @@ export interface ControlStates { export interface LogItem { callId: Call['id']; status: Call['status']; + parentId?: Call['id']; } export interface Payload { From d7e529337a25e5706aff79f72c5a3e3ea4b44227 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sun, 12 Jun 2022 12:59:49 +0200 Subject: [PATCH 03/19] Fix mock data --- addons/interactions/src/mocks/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/interactions/src/mocks/index.ts b/addons/interactions/src/mocks/index.ts index 7952b6ecf89..8113f28e26c 100644 --- a/addons/interactions/src/mocks/index.ts +++ b/addons/interactions/src/mocks/index.ts @@ -24,7 +24,7 @@ export const getCall = (status: CallStates): Call => { }; const overrides = CallStates.ERROR - ? { exception: { name: 'Error', stack: '', message: "Things didn't work!" } } + ? { exception: { name: 'Error', stack: '', message: 'Oops!', callId: defaultData.id } } : {}; return { ...defaultData, ...overrides }; From b06e237bbd0faabc2383b0441b3da6349837e9c7 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sun, 12 Jun 2022 20:16:21 +0200 Subject: [PATCH 04/19] Fix exception type --- lib/instrumenter/src/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/instrumenter/src/types.ts b/lib/instrumenter/src/types.ts index 862036ea75a..e88eb4de79e 100644 --- a/lib/instrumenter/src/types.ts +++ b/lib/instrumenter/src/types.ts @@ -11,7 +11,7 @@ export interface Call { interceptable: boolean; retain: boolean; status?: CallStates.DONE | CallStates.ERROR | CallStates.ACTIVE | CallStates.WAITING; - exception?: Error; + exception?: Error & { callId: Call['id'] }; } export enum CallStates { From f95a9263e0c74b0f019f5e175fdaf87c87008128 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Sun, 12 Jun 2022 20:27:43 +0200 Subject: [PATCH 05/19] Disable interaction on child interactions --- addons/interactions/src/components/Interaction/Interaction.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/interactions/src/components/Interaction/Interaction.tsx b/addons/interactions/src/components/Interaction/Interaction.tsx index 9f96c9a9585..21f4d4450d4 100644 --- a/addons/interactions/src/components/Interaction/Interaction.tsx +++ b/addons/interactions/src/components/Interaction/Interaction.tsx @@ -79,7 +79,7 @@ export const Interaction = ({ controls.goto(call.id)} - disabled={!controlStates.goto} + disabled={!controlStates.goto || !!call.parentId} onMouseEnter={() => controlStates.goto && setIsHovered(true)} onMouseLeave={() => controlStates.goto && setIsHovered(false)} > From 1505bbc9b6f8b7930be97c086f6fb9c2974589e9 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Mon, 13 Jun 2022 23:32:08 +0200 Subject: [PATCH 06/19] Many fixes --- addons/interactions/src/Panel.tsx | 25 +++-- .../components/Interaction/Interaction.tsx | 78 +++++++++++----- .../src/components/MatcherResult.tsx | 2 +- .../src/components/MethodCall.stories.tsx | 57 ++++++++---- .../src/components/MethodCall.tsx | 89 +++++++++--------- .../src/components/Subnav/Subnav.stories.tsx | 1 + lib/instrumenter/src/instrumenter.ts | 92 ++++++++++--------- lib/instrumenter/src/types.ts | 7 +- 8 files changed, 212 insertions(+), 139 deletions(-) diff --git a/addons/interactions/src/Panel.tsx b/addons/interactions/src/Panel.tsx index 4693f857799..e1fd3a1f6f8 100644 --- a/addons/interactions/src/Panel.tsx +++ b/addons/interactions/src/Panel.tsx @@ -29,6 +29,7 @@ interface InteractionsPanelProps { controls: Controls; controlStates: ControlStates; interactions: (Call & { status?: CallStates })[]; + nextCallId?: Call['id']; fileName?: string; hasException?: boolean; isPlaying?: boolean; @@ -63,6 +64,7 @@ export const AddonPanelPure: React.FC = React.memo( controls, controlStates, interactions, + nextCallId, fileName, hasException, isPlaying, @@ -87,15 +89,18 @@ export const AddonPanelPure: React.FC = React.memo( setIsRerunAnimating={setIsRerunAnimating} /> )} - {interactions.map((call) => ( - - ))} +
+ {interactions.map((call) => ( + + ))} +
{!isPlaying && interactions.length === 0 && ( @@ -176,6 +181,7 @@ export const Panel: React.FC = (props) => { const showStatus = log.length > 0 && !isPlaying; const hasException = log.some((item) => item.status === CallStates.ERROR); + const nextCall = log.find((item) => item.status === CallStates.WAITING && !item.parentId); return ( @@ -188,6 +194,7 @@ export const Panel: React.FC = (props) => { controls={controls} controlStates={controlStates} interactions={interactions} + nextCallId={nextCall?.callId} fileName={fileName} hasException={hasException} isPlaying={isPlaying} diff --git a/addons/interactions/src/components/Interaction/Interaction.tsx b/addons/interactions/src/components/Interaction/Interaction.tsx index 21f4d4450d4..34b68808c29 100644 --- a/addons/interactions/src/components/Interaction/Interaction.tsx +++ b/addons/interactions/src/components/Interaction/Interaction.tsx @@ -17,18 +17,30 @@ const MethodCallWrapper = styled.div(() => ({ const RowContainer = styled('div', { shouldForwardProp: (prop) => !['call'].includes(prop) })<{ call: Call; -}>(({ theme, call }) => ({ - display: 'flex', - flexDirection: 'column', - borderBottom: `1px solid ${theme.appBorderColor}`, - fontFamily: typography.fonts.base, - fontSize: 13, - ...(call.status === CallStates.ERROR && { - backgroundColor: - theme.base === 'dark' ? transparentize(0.93, theme.color.negative) : theme.background.warning, + isNext: boolean; +}>( + ({ theme, call }) => ({ + display: 'flex', + flexDirection: 'column', + borderBottom: `1px solid ${theme.appBorderColor}`, + fontFamily: typography.fonts.base, + fontSize: 13, + ...(call.status === CallStates.ERROR && { + backgroundColor: + theme.base === 'dark' + ? transparentize(0.93, theme.color.negative) + : theme.background.warning, + }), + paddingLeft: call.parentId ? 20 : 0, }), - paddingLeft: call.parentId ? 20 : 0, -})); + ({ theme, isNext }) => + isNext && { + '&::before': { + content: '""', + boxShadow: `0 0 1px 1px ${theme.color.secondary}`, + }, + } +); const RowLabel = styled('button', { shouldForwardProp: (prop) => !['call'].includes(prop) })< React.ButtonHTMLAttributes & { call: Call } @@ -56,30 +68,52 @@ const RowLabel = styled('button', { shouldForwardProp: (prop) => !['call'].inclu }, })); -const RowMessage = styled('pre')({ - margin: 0, - padding: '8px 10px 8px 30px', +const RowMessage = styled('div')(({ theme }) => ({ + padding: '8px 10px 8px 36px', fontSize: typography.size.s1, -}); + pre: { + margin: 0, + padding: 0, + }, + p: { + color: theme.color.dark, + }, +})); + +const Exception = ({ exception }: { exception: Call['exception'] }) => { + if (exception.message.startsWith('expect(')) { + return ; + } + const paragraphs = exception.message.split('\n\n'); + const more = paragraphs.length > 1; + return ( + + 
{paragraphs[0]}
+ {more &&

See the full stack trace in the browser console.

} + + ); +}; export const Interaction = ({ call, callsById, controls, controlStates, + nextCallId, }: { call: Call; callsById: Map; controls: Controls; controlStates: ControlStates; + nextCallId: Call['id']; }) => { const [isHovered, setIsHovered] = React.useState(false); return ( - + controls.goto(call.id)} - disabled={!controlStates.goto || !!call.parentId} + disabled={!controlStates.goto || !call.interceptable || !!call.parentId} onMouseEnter={() => controlStates.goto && setIsHovered(true)} onMouseLeave={() => controlStates.goto && setIsHovered(false)} > @@ -88,13 +122,9 @@ export const Interaction = ({ - {call.status === CallStates.ERROR && - call.exception?.callId === call.id && - (call.exception.message.startsWith('expect(') ? ( - - ) : ( - {call.exception.message} - ))} + {call.status === CallStates.ERROR && call.exception?.callId === call.id && ( + + )} ); }; diff --git a/addons/interactions/src/components/MatcherResult.tsx b/addons/interactions/src/components/MatcherResult.tsx index cf52a920ae4..4e3814702e3 100644 --- a/addons/interactions/src/components/MatcherResult.tsx +++ b/addons/interactions/src/components/MatcherResult.tsx @@ -50,7 +50,7 @@ export const MatcherResult = ({ message }: { message: string }) => { 
diff --git a/addons/interactions/src/components/MethodCall.stories.tsx b/addons/interactions/src/components/MethodCall.stories.tsx
index feda43c8ff5..b7551d0c32e 100644
--- a/addons/interactions/src/components/MethodCall.stories.tsx
+++ b/addons/interactions/src/components/MethodCall.stories.tsx
@@ -27,7 +27,6 @@ export default {
   },
 };
 
-class FooBar {}
 export const Args = () => (
   

     
@@ -56,37 +55,50 @@ export const Args = () => (
       }}
       showObjectInspector
     />
-    
-    
+    
+    
     
     
     
     
     
-    
-    
-    
-    
-    
-    
-    
     
-    
-    
+    
+    
+    
+    
+    
+    
+    
+    
+    
     {/* eslint-disable-next-line symbol-description */}
-    
-    
+    
+    
   

 );
 
 const calls: Call[] = [
   {
+    cursor: 0,
     id: '1',
     path: ['screen'],
     method: 'getByText',
@@ -96,6 +108,7 @@ const calls: Call[] = [
     retain: false,
   },
   {
+    cursor: 1,
     id: '2',
     path: ['userEvent'],
     method: 'click',
@@ -105,6 +118,7 @@ const calls: Call[] = [
     retain: false,
   },
   {
+    cursor: 2,
     id: '3',
     path: [],
     method: 'expect',
@@ -114,6 +128,7 @@ const calls: Call[] = [
     retain: false,
   },
   {
+    cursor: 3,
     id: '4',
     path: [{ __callId__: '3' }, 'not'],
     method: 'toBe',
@@ -123,6 +138,7 @@ const calls: Call[] = [
     retain: false,
   },
   {
+    cursor: 4,
     id: '5',
     path: ['jest'],
     method: 'fn',
@@ -132,6 +148,7 @@ const calls: Call[] = [
     retain: false,
   },
   {
+    cursor: 5,
     id: '6',
     path: [],
     method: 'expect',
@@ -141,6 +158,7 @@ const calls: Call[] = [
     retain: false,
   },
   {
+    cursor: 6,
     id: '7',
     path: ['expect'],
     method: 'stringMatching',
@@ -150,6 +168,7 @@ const calls: Call[] = [
     retain: false,
   },
   {
+    cursor: 7,
     id: '8',
     path: [{ __callId__: '6' }, 'not'],
     method: 'toHaveBeenCalledWith',
diff --git a/addons/interactions/src/components/MethodCall.tsx b/addons/interactions/src/components/MethodCall.tsx
index 10a3bf2c0f4..e74e02e6340 100644
--- a/addons/interactions/src/components/MethodCall.tsx
+++ b/addons/interactions/src/components/MethodCall.tsx
@@ -111,32 +111,34 @@ export const Node = ({
       return ;
     case value === undefined:
       return ;
+    case Array.isArray(value):
+      return ;
     case typeof value === 'string':
-      return ;
+      return ;
     case typeof value === 'number':
-      return ;
+      return ;
     case typeof value === 'boolean':
-      return ;
-    case typeof value === 'function':
-      return ;
-    case value instanceof Array:
-      return ;
-    case value instanceof Date:
-      return ;
-    case value instanceof Error:
-      return ;
-    case value instanceof RegExp:
-      return ;
+      return ;
+
+    /* eslint-disable no-underscore-dangle */
+    case Object.prototype.hasOwnProperty.call(value, '__date__'):
+      return ;
+    case Object.prototype.hasOwnProperty.call(value, '__error__'):
+      return ;
+    case Object.prototype.hasOwnProperty.call(value, '__regexp__'):
+      return ;
+    case Object.prototype.hasOwnProperty.call(value, '__function__'):
+      return ;
+    case Object.prototype.hasOwnProperty.call(value, '__symbol__'):
+      return ;
     case Object.prototype.hasOwnProperty.call(value, '__element__'):
-      // eslint-disable-next-line no-underscore-dangle
-      return ;
+      return ;
+    case Object.prototype.hasOwnProperty.call(value, '__class__'):
+      return ;
     case Object.prototype.hasOwnProperty.call(value, '__callId__'):
-      // eslint-disable-next-line no-underscore-dangle
       return ;
-    case typeof value === 'object' &&
-      value.constructor?.name &&
-      value.constructor?.name !== 'Object':
-      return ;
+    /* eslint-enable no-underscore-dangle */
+
     case Object.prototype.toString.call(value) === '[object Object]':
       return ;
     default:
@@ -263,18 +265,23 @@ export const ObjectNode = ({
   );
 };
 
-export const ClassNode = ({ value }: { value: Record }) => {
+export const ClassNode = ({ name }: { name: string }) => {
   const colors = useThemeColors();
-  return {value.constructor.name};
+  return {name};
 };
 
-export const FunctionNode = ({ value }: { value: Function }) => {
+export const FunctionNode = ({ name }: { name: string }) => {
   const colors = useThemeColors();
-  return {value.name || 'anonymous'};
+  return {name || anonymous};
 };
 
-export const ElementNode = ({ value }: { value: ElementRef['__element__'] }) => {
-  const { prefix, localName, id, classNames = [], innerText } = value;
+export const ElementNode = ({
+  prefix,
+  localName,
+  id,
+  classNames = [],
+  innerText,
+}: ElementRef['__element__']) => {
   const name = prefix ? `${prefix}:${localName}` : localName;
   const colors = useThemeColors();
   return (
@@ -309,8 +316,8 @@ export const ElementNode = ({ value }: { value: ElementRef['__element__'] }) =>
   );
 };
 
-export const DateNode = ({ value }: { value: Date }) => {
-  const [date, time, ms] = value.toISOString().split(/[T.Z]/);
+export const DateNode = ({ value }: { value: string }) => {
+  const [date, time, ms] = value.split(/[T.Z]/);
   const colors = useThemeColors();
   return (
     
@@ -323,42 +330,36 @@ export const DateNode = ({ value }: { value: Date }) => {
   );
 };
 
-export const ErrorNode = ({ value }: { value: Error }) => {
+export const ErrorNode = ({ name, message }: { name: string; message: string }) => {
   const colors = useThemeColors();
   return (
     
-      {value.name}
-      {value.message && ': '}
-      {value.message && (
-         50 ? value.message : ''}
-        >
-          {ellipsize(value.message, 50)}
+      {name}
+      {message && ': '}
+      {message && (
+         50 ? message : ''}>
+          {ellipsize(message, 50)}
         
       )}
     
   );
 };
 
-export const RegExpNode = ({ value }: { value: RegExp }) => {
+export const RegExpNode = ({ flags, source }: { flags: string; source: string }) => {
   const colors = useThemeColors();
   return (
     
-      /{value.source}/{value.flags}
+      /{source}/{flags}
     
   );
 };
 
-export const SymbolNode = ({ value }: { value: symbol }) => {
+export const SymbolNode = ({ description }: { description: string }) => {
   const colors = useThemeColors();
   return (
     
       Symbol(
-      {value.description && (
-        {JSON.stringify(value.description)}
-      )}
-      )
+      {description && "{description}"})
     
   );
 };
diff --git a/addons/interactions/src/components/Subnav/Subnav.stories.tsx b/addons/interactions/src/components/Subnav/Subnav.stories.tsx
index 9605bc53252..40e365d03fb 100644
--- a/addons/interactions/src/components/Subnav/Subnav.stories.tsx
+++ b/addons/interactions/src/components/Subnav/Subnav.stories.tsx
@@ -12,6 +12,7 @@ export default {
       goto: action('goto'),
       next: action('next'),
       end: action('end'),
+      rerun: action('rerun'),
     },
     controlStates: {
       debugger: true,
diff --git a/lib/instrumenter/src/instrumenter.ts b/lib/instrumenter/src/instrumenter.ts
index 06b07b89360..a4302ac750a 100644
--- a/lib/instrumenter/src/instrumenter.ts
+++ b/lib/instrumenter/src/instrumenter.ts
@@ -10,7 +10,7 @@ import {
 } from '@storybook/core-events';
 import global from 'global';
 
-import { Call, CallRef, CallStates, State, Options, ControlStates, LogItem } from './types';
+import { Call, CallRef, CallStates, ControlStates, LogItem, Options, State } from './types';
 
 export const EVENTS = {
   CALL: 'instrumenter/call',
@@ -73,7 +73,6 @@ const getInitialState = (): State => ({
   playUntil: undefined,
   resolvers: {},
   syncTimeout: undefined,
-  forwardedException: undefined,
 });
 
 const getRetainedState = (state: State, isDebugging = false) => {
@@ -132,7 +131,7 @@ export class Instrumenter {
 
     // Start with a clean slate before playing after a remount, and stop debugging when done.
     this.channel.on(STORY_RENDER_PHASE_CHANGED, ({ storyId, newPhase }) => {
-      const { isDebugging, forwardedException } = this.getState(storyId);
+      const { isDebugging } = this.getState(storyId);
       this.setState(storyId, { renderPhase: newPhase });
       if (newPhase === 'playing') {
         resetState({ storyId, isDebugging });
@@ -142,10 +141,7 @@ export class Instrumenter {
           isLocked: false,
           isPlaying: false,
           isDebugging: false,
-          forwardedException: undefined,
         });
-        // Rethrow any unhandled forwarded exception so it doesn't go unnoticed.
-        if (forwardedException) throw forwardedException;
       }
       if (newPhase === 'errored') {
         this.setState(storyId, {
@@ -189,12 +185,11 @@ export class Instrumenter {
 
     const back = ({ storyId }: { storyId: string }) => {
       const { isDebugging } = this.getState(storyId);
-      const log = this.getLog(storyId);
+      const log = this.getLog(storyId).filter((call) => !call.parentId);
       const next = isDebugging
         ? log.findIndex(({ status }) => status === CallStates.WAITING)
         : log.length;
-      const playUntil = log[next - 2]?.parentId || log[next - 2]?.callId;
-      start({ storyId, playUntil });
+      start({ storyId, playUntil: log[next - 2]?.callId });
     };
 
     const goto = ({ storyId, callId }: { storyId: string; callId: Call['id'] }) => {
@@ -276,7 +271,7 @@ export class Instrumenter {
           seen.add((node as CallRef).__callId__);
         }
       });
-      if (call.interceptable && !seen.has(call.id)) {
+      if ((call.interceptable || call.exception) && !seen.has(call.id)) {
         acc.unshift({ callId: call.id, status: call.status, parentId: call.parentId });
         seen.add(call.id);
       }
@@ -378,25 +373,50 @@ export class Instrumenter {
     // const { abortSignal } = global.window.__STORYBOOK_PREVIEW__ || {};
     // if (abortSignal && abortSignal.aborted) throw IGNORED_EXCEPTION;
 
-    const { callRefsByResult, forwardedException, renderPhase } = this.getState(call.storyId);
+    const { callRefsByResult, renderPhase } = this.getState(call.storyId);
 
-    const info: Call = {
-      ...call,
-      // Map args that originate from a tracked function call to a call reference to enable nesting.
-      // These values are often not fully serializable anyway (e.g. HTML elements).
-      args: call.args.map((arg) => {
-        if (callRefsByResult.has(arg)) {
-          return callRefsByResult.get(arg);
-        }
-        if (arg instanceof global.window.HTMLElement) {
-          const { prefix, localName, id, classList, innerText } = arg;
-          const classNames = Array.from(classList);
-          return { __element__: { prefix, localName, id, classNames, innerText } };
-        }
-        return arg;
-      }),
+    // Map complex values to a JSON-serializable representation.
+    const serializeValues = (value: any): any => {
+      if (callRefsByResult.has(value)) {
+        return callRefsByResult.get(value);
+      }
+      if (value instanceof Array) {
+        return value.map(serializeValues);
+      }
+      if (value instanceof Date) {
+        return { __date__: { value: value.toISOString() } };
+      }
+      if (value instanceof Error) {
+        const { name, message, stack } = value;
+        return { __error__: { name, message, stack } };
+      }
+      if (value instanceof RegExp) {
+        const { flags, source } = value;
+        return { __regexp__: { flags, source } };
+      }
+      if (value instanceof global.window.HTMLElement) {
+        const { prefix, localName, id, classList, innerText } = value;
+        const classNames = Array.from(classList);
+        return { __element__: { prefix, localName, id, classNames, innerText } };
+      }
+      if (typeof value === 'function') {
+        return { __function__: { name: value.name } };
+      }
+      if (typeof value === 'symbol') {
+        return { __symbol__: { description: value.description } };
+      }
+      if (
+        typeof value === 'object' &&
+        value.constructor?.name &&
+        value.constructor?.name !== 'Object'
+      ) {
+        return { __class__: { name: value.constructor.name } };
+      }
+      return value;
     };
 
+    const info: Call = { ...call, args: call.args.map(serializeValues) };
+
     // Mark any ancestor calls as "chained upon" so we won't attempt to defer it later.
     call.path.forEach((ref: any) => {
       if (ref?.__callId__) {
@@ -408,7 +428,7 @@ export class Instrumenter {
 
     const handleException = (e: any) => {
       if (e instanceof Error) {
-        const { name, message, stack, callId = call.id } = e as Error & { callId?: Call['id'] };
+        const { name, message, stack, callId = call.id } = e as Error & { callId: Call['id'] };
         const exception = { name, message, stack, callId };
         this.update({ ...info, status: CallStates.ERROR, exception });
 
@@ -420,7 +440,7 @@ export class Instrumenter {
           ]),
         }));
 
-        // Exceptions inside callbacks should bubble up to the parent call rather than be forwarded.
+        // Exceptions inside callbacks should bubble up to the parent call.
         if (call.parentId) {
           Object.defineProperty(e, 'callId', { value: call.id });
           throw e;
@@ -428,25 +448,16 @@ export class Instrumenter {
 
         // We need to throw to break out of the play function, but we don't want to trigger a redbox
         // so we throw an ignoredException, which is caught and silently ignored by Storybook.
-        if (call.interceptable && e !== alreadyCompletedException) {
+        if (e !== alreadyCompletedException) {
+          // eslint-disable-next-line no-console
+          console?.warn(e);
           throw IGNORED_EXCEPTION;
         }
-
-        // Non-interceptable calls need their exceptions forwarded to the next interceptable call.
-        // In case no interceptable call picks it up, it'll get rethrown in the "completed" phase.
-        this.setState(call.storyId, { forwardedException: e });
-        return e;
       }
       throw e;
     };
 
     try {
-      // An earlier, non-interceptable call might have forwarded an exception.
-      if (forwardedException) {
-        this.setState(call.storyId, { forwardedException: undefined });
-        throw forwardedException;
-      }
-
       if (renderPhase === 'played' && !call.retain) {
         throw alreadyCompletedException;
       }
@@ -462,7 +473,6 @@ export class Instrumenter {
         // We only want to wrap plain functions, not objects.
         if (typeof arg !== 'function' || Object.keys(arg).length) return arg;
 
-        // console.log(call.id);
         return (...args: any) => {
           // Set the cursor and parentId for calls that happen inside the callback.
           const { cursor, parentId } = this.getState(call.storyId);
diff --git a/lib/instrumenter/src/types.ts b/lib/instrumenter/src/types.ts
index e88eb4de79e..e9cca700354 100644
--- a/lib/instrumenter/src/types.ts
+++ b/lib/instrumenter/src/types.ts
@@ -11,7 +11,12 @@ export interface Call {
   interceptable: boolean;
   retain: boolean;
   status?: CallStates.DONE | CallStates.ERROR | CallStates.ACTIVE | CallStates.WAITING;
-  exception?: Error & { callId: Call['id'] };
+  exception?: {
+    name: Error['name'];
+    message: Error['message'];
+    stack: Error['stack'];
+    callId: Call['id'];
+  };
 }
 
 export enum CallStates {

From 52cb7bdc589f3624e7b4367ac775b58e9781583b Mon Sep 17 00:00:00 2001
From: Gert Hengeveld 
Date: Tue, 14 Jun 2022 08:43:00 +0200
Subject: [PATCH 07/19] Remove unused eslint directives

---
 addons/interactions/src/components/MethodCall.stories.tsx | 1 -
 lib/instrumenter/src/instrumenter.ts                      | 1 -
 2 files changed, 2 deletions(-)

diff --git a/addons/interactions/src/components/MethodCall.stories.tsx b/addons/interactions/src/components/MethodCall.stories.tsx
index b7551d0c32e..dda96949202 100644
--- a/addons/interactions/src/components/MethodCall.stories.tsx
+++ b/addons/interactions/src/components/MethodCall.stories.tsx
@@ -90,7 +90,6 @@ export const Args = () => (
     />
     
     
-    {/* eslint-disable-next-line symbol-description */}
diff --git a/lib/instrumenter/src/instrumenter.ts b/lib/instrumenter/src/instrumenter.ts index a4302ac750a..08133599074 100644 --- a/lib/instrumenter/src/instrumenter.ts +++ b/lib/instrumenter/src/instrumenter.ts @@ -449,7 +449,6 @@ export class Instrumenter { // We need to throw to break out of the play function, but we don't want to trigger a redbox // so we throw an ignoredException, which is caught and silently ignored by Storybook. if (e !== alreadyCompletedException) { - // eslint-disable-next-line no-console console?.warn(e); throw IGNORED_EXCEPTION; } From c1ea010c832774a632a3742374246433e60d849d Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 14 Jun 2022 08:46:50 +0200 Subject: [PATCH 08/19] Fix prop forwarding --- .../src/components/Interaction/Interaction.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/addons/interactions/src/components/Interaction/Interaction.tsx b/addons/interactions/src/components/Interaction/Interaction.tsx index 34b68808c29..b2c304187ff 100644 --- a/addons/interactions/src/components/Interaction/Interaction.tsx +++ b/addons/interactions/src/components/Interaction/Interaction.tsx @@ -15,10 +15,9 @@ const MethodCallWrapper = styled.div(() => ({ inlineSize: 'calc( 100% - 40px )', })); -const RowContainer = styled('div', { shouldForwardProp: (prop) => !['call'].includes(prop) })<{ - call: Call; - isNext: boolean; -}>( +const RowContainer = styled('div', { + shouldForwardProp: (prop) => !['call', 'isNext'].includes(prop), +})<{ call: Call; isNext: boolean }>( ({ theme, call }) => ({ display: 'flex', flexDirection: 'column', From 316a543643674c80fcd1ca37556a66146e031de6 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 14 Jun 2022 13:07:03 +0200 Subject: [PATCH 09/19] Fix back button --- lib/instrumenter/src/instrumenter.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/instrumenter/src/instrumenter.ts b/lib/instrumenter/src/instrumenter.ts index 08133599074..5c2001031bb 100644 --- a/lib/instrumenter/src/instrumenter.ts +++ b/lib/instrumenter/src/instrumenter.ts @@ -184,12 +184,12 @@ export class Instrumenter { }; const back = ({ storyId }: { storyId: string }) => { - const { isDebugging } = this.getState(storyId); const log = this.getLog(storyId).filter((call) => !call.parentId); - const next = isDebugging - ? log.findIndex(({ status }) => status === CallStates.WAITING) - : log.length; - start({ storyId, playUntil: log[next - 2]?.callId }); + const last = log.reduceRight((res, item, index) => { + if (res >= 0 || item.status === CallStates.WAITING) return res; + return index; + }, -1); + start({ storyId, playUntil: log[last - 1]?.callId }); }; const goto = ({ storyId, callId }: { storyId: string; callId: Call['id'] }) => { From 069546eb028bde47cf81402a40c768213aa1c95c Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 14 Jun 2022 13:50:30 +0200 Subject: [PATCH 10/19] Fix pausedAt indicator --- addons/interactions/src/Panel.tsx | 12 +++++++----- .../src/components/Interaction/Interaction.tsx | 16 ++++++++-------- lib/instrumenter/src/instrumenter.ts | 6 +++++- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/addons/interactions/src/Panel.tsx b/addons/interactions/src/Panel.tsx index e1fd3a1f6f8..2778d1c2f1d 100644 --- a/addons/interactions/src/Panel.tsx +++ b/addons/interactions/src/Panel.tsx @@ -29,10 +29,10 @@ interface InteractionsPanelProps { controls: Controls; controlStates: ControlStates; interactions: (Call & { status?: CallStates })[]; - nextCallId?: Call['id']; fileName?: string; hasException?: boolean; isPlaying?: boolean; + pausedAt?: Call['id']; calls: Map; endRef?: React.Ref; onScrollToEnd?: () => void; @@ -64,10 +64,10 @@ export const AddonPanelPure: React.FC = React.memo( controls, controlStates, interactions, - nextCallId, fileName, hasException, isPlaying, + pausedAt, onScrollToEnd, endRef, isRerunAnimating, @@ -97,7 +97,7 @@ export const AddonPanelPure: React.FC = React.memo( callsById={calls} controls={controls} controlStates={controlStates} - nextCallId={nextCallId} + pausedAt={pausedAt} /> ))} @@ -121,6 +121,7 @@ export const AddonPanelPure: React.FC = React.memo( export const Panel: React.FC = (props) => { const [storyId, setStoryId] = React.useState(); const [controlStates, setControlStates] = React.useState(INITIAL_CONTROL_STATES); + const [pausedAt, setPausedAt] = React.useState(); const [isPlaying, setPlaying] = React.useState(false); const [isRerunAnimating, setIsRerunAnimating] = React.useState(false); const [scrollTarget, setScrollTarget] = React.useState(); @@ -151,10 +152,12 @@ export const Panel: React.FC = (props) => { [EVENTS.SYNC]: (payload) => { setControlStates(payload.controlStates); setLog(payload.logItems); + setPausedAt(payload.pausedAt); }, [STORY_RENDER_PHASE_CHANGED]: (event) => { setStoryId(event.storyId); setPlaying(event.newPhase === 'playing'); + setPausedAt(undefined); }, }, [] @@ -181,7 +184,6 @@ export const Panel: React.FC = (props) => { const showStatus = log.length > 0 && !isPlaying; const hasException = log.some((item) => item.status === CallStates.ERROR); - const nextCall = log.find((item) => item.status === CallStates.WAITING && !item.parentId); return ( @@ -194,10 +196,10 @@ export const Panel: React.FC = (props) => { controls={controls} controlStates={controlStates} interactions={interactions} - nextCallId={nextCall?.callId} fileName={fileName} hasException={hasException} isPlaying={isPlaying} + pausedAt={pausedAt} endRef={endRef} onScrollToEnd={scrollTarget && scrollToTarget} isRerunAnimating={isRerunAnimating} diff --git a/addons/interactions/src/components/Interaction/Interaction.tsx b/addons/interactions/src/components/Interaction/Interaction.tsx index b2c304187ff..0ca32dfd2f7 100644 --- a/addons/interactions/src/components/Interaction/Interaction.tsx +++ b/addons/interactions/src/components/Interaction/Interaction.tsx @@ -16,8 +16,8 @@ const MethodCallWrapper = styled.div(() => ({ })); const RowContainer = styled('div', { - shouldForwardProp: (prop) => !['call', 'isNext'].includes(prop), -})<{ call: Call; isNext: boolean }>( + shouldForwardProp: (prop) => !['call', 'pausedAt'].includes(prop), +})<{ call: Call; pausedAt: Call['id'] }>( ({ theme, call }) => ({ display: 'flex', flexDirection: 'column', @@ -32,11 +32,11 @@ const RowContainer = styled('div', { }), paddingLeft: call.parentId ? 20 : 0, }), - ({ theme, isNext }) => - isNext && { + ({ theme, call, pausedAt }) => + pausedAt === call.id && { '&::before': { content: '""', - boxShadow: `0 0 1px 1px ${theme.color.secondary}`, + boxShadow: `0 0 0 1px ${theme.color.warning}`, }, } ); @@ -98,17 +98,17 @@ export const Interaction = ({ callsById, controls, controlStates, - nextCallId, + pausedAt, }: { call: Call; callsById: Map; controls: Controls; controlStates: ControlStates; - nextCallId: Call['id']; + pausedAt?: Call['id']; }) => { const [isHovered, setIsHovered] = React.useState(false); return ( - + controls.goto(call.id)} diff --git a/lib/instrumenter/src/instrumenter.ts b/lib/instrumenter/src/instrumenter.ts index 5c2001031bb..ea3c8a57a51 100644 --- a/lib/instrumenter/src/instrumenter.ts +++ b/lib/instrumenter/src/instrumenter.ts @@ -544,6 +544,9 @@ export class Instrumenter { sync(storyId: StoryId) { const { isLocked, isPlaying } = this.getState(storyId); const logItems: LogItem[] = this.getLog(storyId); + const pausedAt = logItems + .filter(({ parentId }) => !parentId) + .find((item) => item.status === CallStates.WAITING)?.callId; const hasActive = logItems.some((item) => item.status === CallStates.ACTIVE); if (debuggerDisabled || isLocked || hasActive || logItems.length === 0) { @@ -562,7 +565,8 @@ export class Instrumenter { next: isPlaying, end: isPlaying, }; - this.channel.emit(EVENTS.SYNC, { controlStates, logItems }); + + this.channel.emit(EVENTS.SYNC, { controlStates, logItems, pausedAt }); } } From 172da319a7844361faeb6c14a769211f355ba4b3 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 14 Jun 2022 13:50:52 +0200 Subject: [PATCH 11/19] Fix broken play function --- .../src/components/AccountForm/addon-interactions.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/interactions/src/components/AccountForm/addon-interactions.stories.tsx b/addons/interactions/src/components/AccountForm/addon-interactions.stories.tsx index 9619a228b50..d3df3f635f2 100644 --- a/addons/interactions/src/components/AccountForm/addon-interactions.stories.tsx +++ b/addons/interactions/src/components/AccountForm/addon-interactions.stories.tsx @@ -131,7 +131,7 @@ export const StandardEmailFailed: CSF3Story = { await userEvent.click(canvas.getByRole('button', { name: /create account/i })); await canvas.findByText('Please enter a correctly formatted email address'); - expect(args.onSubmit).not.toHaveBeenCalled(); + await expect(args.onSubmit).not.toHaveBeenCalled(); }, }; From db398c2d13d0884a979f06b7c74acb72d44532b7 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 14 Jun 2022 14:18:17 +0200 Subject: [PATCH 12/19] Value might be null --- lib/instrumenter/src/instrumenter.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/instrumenter/src/instrumenter.ts b/lib/instrumenter/src/instrumenter.ts index ea3c8a57a51..230dd9bc681 100644 --- a/lib/instrumenter/src/instrumenter.ts +++ b/lib/instrumenter/src/instrumenter.ts @@ -407,8 +407,8 @@ export class Instrumenter { } if ( typeof value === 'object' && - value.constructor?.name && - value.constructor?.name !== 'Object' + value?.constructor?.name && + value?.constructor?.name !== 'Object' ) { return { __class__: { name: value.constructor.name } }; } From 5fa404334c3c98f3cbc7d81eaee637e41f962132 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 14 Jun 2022 22:05:06 +0200 Subject: [PATCH 13/19] Tweak indicator style --- .../src/components/Interaction/Interaction.tsx | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/addons/interactions/src/components/Interaction/Interaction.tsx b/addons/interactions/src/components/Interaction/Interaction.tsx index 0ca32dfd2f7..45e6fab4d9a 100644 --- a/addons/interactions/src/components/Interaction/Interaction.tsx +++ b/addons/interactions/src/components/Interaction/Interaction.tsx @@ -19,6 +19,7 @@ const RowContainer = styled('div', { shouldForwardProp: (prop) => !['call', 'pausedAt'].includes(prop), })<{ call: Call; pausedAt: Call['id'] }>( ({ theme, call }) => ({ + position: 'relative', display: 'flex', flexDirection: 'column', borderBottom: `1px solid ${theme.appBorderColor}`, @@ -36,7 +37,19 @@ const RowContainer = styled('div', { pausedAt === call.id && { '&::before': { content: '""', - boxShadow: `0 0 0 1px ${theme.color.warning}`, + position: 'absolute', + top: -5, + borderTop: '4.5px solid transparent', + borderLeft: `7px solid ${theme.color.warning}`, + borderBottom: '4.5px solid transparent', + }, + '&::after': { + content: '""', + position: 'absolute', + top: -1, + height: 1, + width: '100%', + background: theme.color.warning, }, } ); From 25b0d66b2a69df9e294158ebaac9591f6fd6f873 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 14 Jun 2022 22:24:18 +0200 Subject: [PATCH 14/19] Fix rendering inconsistency --- addons/interactions/src/components/Interaction/Interaction.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/addons/interactions/src/components/Interaction/Interaction.tsx b/addons/interactions/src/components/Interaction/Interaction.tsx index 45e6fab4d9a..f7cd0206091 100644 --- a/addons/interactions/src/components/Interaction/Interaction.tsx +++ b/addons/interactions/src/components/Interaction/Interaction.tsx @@ -47,9 +47,8 @@ const RowContainer = styled('div', { content: '""', position: 'absolute', top: -1, - height: 1, width: '100%', - background: theme.color.warning, + borderTop: `1.5px solid ${theme.color.warning}`, }, } ); From b12186d55ee0ef6f963d09582f7e8be28b68e507 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 14 Jun 2022 22:46:51 +0200 Subject: [PATCH 15/19] Fix zIndex --- addons/interactions/src/components/Interaction/Interaction.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/addons/interactions/src/components/Interaction/Interaction.tsx b/addons/interactions/src/components/Interaction/Interaction.tsx index f7cd0206091..ec1672027aa 100644 --- a/addons/interactions/src/components/Interaction/Interaction.tsx +++ b/addons/interactions/src/components/Interaction/Interaction.tsx @@ -39,6 +39,7 @@ const RowContainer = styled('div', { content: '""', position: 'absolute', top: -5, + zIndex: 1, borderTop: '4.5px solid transparent', borderLeft: `7px solid ${theme.color.warning}`, borderBottom: '4.5px solid transparent', @@ -47,6 +48,7 @@ const RowContainer = styled('div', { content: '""', position: 'absolute', top: -1, + zIndex: 1, width: '100%', borderTop: `1.5px solid ${theme.color.warning}`, }, From ee48cf055b01a847ad3c7e130c2a2c64e771fff9 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 14 Jun 2022 23:47:15 +0200 Subject: [PATCH 16/19] Fix tests --- lib/instrumenter/src/instrumenter.test.ts | 49 +++++++---------------- lib/instrumenter/src/instrumenter.ts | 4 +- 2 files changed, 16 insertions(+), 37 deletions(-) diff --git a/lib/instrumenter/src/instrumenter.test.ts b/lib/instrumenter/src/instrumenter.test.ts index 7599bce6b3a..f0f97b57363 100644 --- a/lib/instrumenter/src/instrumenter.test.ts +++ b/lib/instrumenter/src/instrumenter.test.ts @@ -1,6 +1,7 @@ /* eslint-disable no-underscore-dangle */ import { addons, mockChannel } from '@storybook/addons'; +import { logger } from '@storybook/client-logger'; import { FORCE_REMOUNT, SET_CURRENT_STORY, @@ -11,6 +12,8 @@ import global from 'global'; import { EVENTS, Instrumenter } from './instrumenter'; import type { Options } from './types'; +jest.mock('@storybook/client-logger'); + const callSpy = jest.fn(); const syncSpy = jest.fn(); const forceRemountSpy = jest.fn(); @@ -298,39 +301,40 @@ describe('Instrumenter', () => { ); }); - it('catches thrown errors and returns the error', () => { + it('catches thrown errors and throws an ignoredException instead', () => { const { fn } = instrument({ fn: () => { throw new Error('Boom!'); }, }); - expect(fn()).toEqual(new Error('Boom!')); - expect(() => setRenderPhase('played')).toThrow(new Error('Boom!')); + expect(fn).toThrow('ignoredException'); }); - it('forwards nested exceptions', () => { + it('catches nested exceptions and throws an ignoredException instead', () => { const { fn1, fn2 } = instrument({ - fn1: (...args: any) => {}, // doesn't forward args + fn1: (_: any) => {}, fn2: () => { throw new Error('Boom!'); }, }); - expect(fn1(fn2())).toEqual(new Error('Boom!')); - expect(() => setRenderPhase('played')).toThrow(new Error('Boom!')); + expect(() => fn1(fn2())).toThrow('ignoredException'); }); it('bubbles child exceptions up to parent (in callback)', () => { const { fn1, fn2 } = instrument({ - fn1: (callback?: Function) => callback && callback(), + fn1: jest.fn((callback: Function) => callback()), fn2: () => { throw new Error('Boom!'); }, }); - expect( + expect(() => fn1(() => { fn2(); }) - ).toEqual(new Error('Boom!')); + ).toThrow('ignoredException'); + expect(fn1).toHaveBeenCalled(); + expect(logger.warn).toHaveBeenCalledWith(new Error('Boom!')); + expect((logger.warn as any).mock.calls[0][0].callId).toBe('kind--story [0] fn1 [0] fn2'); }); it("re-throws anything that isn't an error", () => { @@ -434,31 +438,6 @@ describe('Instrumenter', () => { }) ); }); - - it('catches thrown errors and throws an ignoredException instead', () => { - const { fn } = instrument( - { - fn: () => { - throw new Error('Boom!'); - }, - }, - options - ); - expect(fn).toThrow('ignoredException'); - }); - - it('catches forwarded exceptions and throws an ignoredException instead', () => { - const { fn1, fn2 } = instrument( - { - fn1: (_: any) => {}, - fn2: () => { - throw new Error('Boom!'); - }, - }, - options - ); - expect(() => fn1(fn2())).toThrow('ignoredException'); - }); }); describe('while debugging', () => { diff --git a/lib/instrumenter/src/instrumenter.ts b/lib/instrumenter/src/instrumenter.ts index 230dd9bc681..4b8253cd354 100644 --- a/lib/instrumenter/src/instrumenter.ts +++ b/lib/instrumenter/src/instrumenter.ts @@ -1,7 +1,7 @@ /* eslint-disable no-underscore-dangle */ import { addons, Channel } from '@storybook/addons'; import type { StoryId } from '@storybook/addons'; -import { once } from '@storybook/client-logger'; +import { logger, once } from '@storybook/client-logger'; import { FORCE_REMOUNT, IGNORED_EXCEPTION, @@ -449,7 +449,7 @@ export class Instrumenter { // We need to throw to break out of the play function, but we don't want to trigger a redbox // so we throw an ignoredException, which is caught and silently ignored by Storybook. if (e !== alreadyCompletedException) { - console?.warn(e); + logger.warn(e); throw IGNORED_EXCEPTION; } } From 9e3657c15bbf70c3bf72463d5f2df0b9539f9dc8 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 15 Jun 2022 11:00:32 +0200 Subject: [PATCH 17/19] Fix story data --- .../src/components/MethodCall.stories.tsx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/addons/interactions/src/components/MethodCall.stories.tsx b/addons/interactions/src/components/MethodCall.stories.tsx index dda96949202..b65001b4e1c 100644 --- a/addons/interactions/src/components/MethodCall.stories.tsx +++ b/addons/interactions/src/components/MethodCall.stories.tsx @@ -142,7 +142,7 @@ const calls: Call[] = [ path: ['jest'], method: 'fn', storyId: 'kind--story', - args: [function actionHandler() {}], + args: [{ __function__: { name: 'actionHandler' } }], interceptable: false, retain: false, }, @@ -162,7 +162,7 @@ const calls: Call[] = [ path: ['expect'], method: 'stringMatching', storyId: 'kind--story', - args: [/hello/i], + args: [{ __regexp__: { flags: 'i', source: 'hello' } }], interceptable: false, retain: false, }, @@ -172,7 +172,13 @@ const calls: Call[] = [ path: [{ __callId__: '6' }, 'not'], method: 'toHaveBeenCalledWith', storyId: 'kind--story', - args: [{ __callId__: '7' }, new Error("Cannot read property 'foo' of undefined")], + args: [ + { __callId__: '7' }, + [ + { __error__: { name: 'Error', message: "Cannot read property 'foo' of undefined" } }, + { __symbol__: { description: 'Hello world' } }, + ], + ], interceptable: false, retain: false, }, From 12f78848220348c5a379f036cf323d670e4a85ea Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 15 Jun 2022 11:14:24 +0200 Subject: [PATCH 18/19] Show pausedAt indicator in story --- addons/interactions/src/Panel.stories.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/addons/interactions/src/Panel.stories.tsx b/addons/interactions/src/Panel.stories.tsx index 8e22606d4ec..6e3b1eca047 100644 --- a/addons/interactions/src/Panel.stories.tsx +++ b/addons/interactions/src/Panel.stories.tsx @@ -68,6 +68,7 @@ export const Paused: Story = { next: true, end: true, }, + pausedAt: getCall(CallStates.WAITING).id, }, }; From 73416f789265cfa9fafb6acdff90bbb1800937d4 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Wed, 15 Jun 2022 12:11:00 +0200 Subject: [PATCH 19/19] Better story data --- addons/interactions/src/Panel.stories.tsx | 18 ++- .../Interaction/Interaction.stories.tsx | 18 ++- addons/interactions/src/mocks/index.ts | 143 ++++++++++++++---- 3 files changed, 139 insertions(+), 40 deletions(-) diff --git a/addons/interactions/src/Panel.stories.tsx b/addons/interactions/src/Panel.stories.tsx index 6e3b1eca047..cc6f37afdd0 100644 --- a/addons/interactions/src/Panel.stories.tsx +++ b/addons/interactions/src/Panel.stories.tsx @@ -4,7 +4,7 @@ import { ComponentStoryObj, ComponentMeta } from '@storybook/react'; import { CallStates } from '@storybook/instrumenter'; import { styled } from '@storybook/theming'; -import { getCall } from './mocks'; +import { getCalls, getInteractions } from './mocks'; import { AddonPanelPure } from './Panel'; import SubnavStories from './components/Subnav/Subnav.stories'; @@ -20,6 +20,8 @@ const StyledWrapper = styled.div(({ theme }) => ({ overflow: 'auto', })); +const interactions = getInteractions(CallStates.DONE); + export default { title: 'Addons/Interactions/Panel', component: AddonPanelPure, @@ -34,10 +36,10 @@ export default { layout: 'fullscreen', }, args: { - calls: new Map(), + calls: new Map(getCalls(CallStates.DONE).map((call) => [call.id, call])), controls: SubnavStories.args.controls, controlStates: SubnavStories.args.controlStates, - interactions: [getCall(CallStates.DONE)], + interactions, fileName: 'addon-interactions.stories.tsx', hasException: false, isPlaying: false, @@ -52,14 +54,14 @@ type Story = ComponentStoryObj; export const Passing: Story = { args: { - interactions: [getCall(CallStates.DONE)], + interactions: getInteractions(CallStates.DONE), }, }; export const Paused: Story = { args: { isPlaying: true, - interactions: [getCall(CallStates.WAITING)], + interactions: getInteractions(CallStates.WAITING), controlStates: { debugger: true, start: false, @@ -68,21 +70,21 @@ export const Paused: Story = { next: true, end: true, }, - pausedAt: getCall(CallStates.WAITING).id, + pausedAt: interactions[interactions.length - 1].id, }, }; export const Playing: Story = { args: { isPlaying: true, - interactions: [getCall(CallStates.ACTIVE)], + interactions: getInteractions(CallStates.ACTIVE), }, }; export const Failed: Story = { args: { hasException: true, - interactions: [getCall(CallStates.ERROR)], + interactions: getInteractions(CallStates.ERROR), }, }; diff --git a/addons/interactions/src/components/Interaction/Interaction.stories.tsx b/addons/interactions/src/components/Interaction/Interaction.stories.tsx index 3fc5ab83b5e..2089cef4a2a 100644 --- a/addons/interactions/src/components/Interaction/Interaction.stories.tsx +++ b/addons/interactions/src/components/Interaction/Interaction.stories.tsx @@ -2,7 +2,7 @@ import { ComponentStoryObj, ComponentMeta } from '@storybook/react'; import { expect } from '@storybook/jest'; import { CallStates } from '@storybook/instrumenter'; import { userEvent, within } from '@storybook/testing-library'; -import { getCall } from '../../mocks'; +import { getCalls } from '../../mocks'; import { Interaction } from './Interaction'; import SubnavStories from '../Subnav/Subnav.stories'; @@ -13,7 +13,7 @@ export default { title: 'Addons/Interactions/Interaction', component: Interaction, args: { - callsById: new Map(), + callsById: new Map(getCalls(CallStates.DONE).map((call) => [call.id, call])), controls: SubnavStories.args.controls, controlStates: SubnavStories.args.controlStates, }, @@ -21,25 +21,31 @@ export default { export const Active: Story = { args: { - call: getCall(CallStates.ACTIVE), + call: getCalls(CallStates.ACTIVE).slice(-1)[0], }, }; export const Waiting: Story = { args: { - call: getCall(CallStates.WAITING), + call: getCalls(CallStates.WAITING).slice(-1)[0], }, }; export const Failed: Story = { args: { - call: getCall(CallStates.ERROR), + call: getCalls(CallStates.ERROR).slice(-1)[0], }, }; export const Done: Story = { args: { - call: getCall(CallStates.DONE), + call: getCalls(CallStates.DONE).slice(-1)[0], + }, +}; + +export const WithParent: Story = { + args: { + call: { ...getCalls(CallStates.DONE).slice(-1)[0], parentId: 'parent-id' }, }, }; diff --git a/addons/interactions/src/mocks/index.ts b/addons/interactions/src/mocks/index.ts index 8113f28e26c..e55fe4d88b8 100644 --- a/addons/interactions/src/mocks/index.ts +++ b/addons/interactions/src/mocks/index.ts @@ -1,31 +1,122 @@ import { CallStates, Call } from '@storybook/instrumenter'; -export const getCall = (status: CallStates): Call => { - const defaultData = { - id: 'addons-interactions-accountform--standard-email-filled [3] change', - cursor: 0, - path: ['fireEvent'], - method: 'change', - storyId: 'addons-interactions-accountform--standard-email-filled', - args: [ - { - __callId__: 'addons-interactions-accountform--standard-email-filled [2] getByTestId', - retain: false, - }, - { - target: { - value: 'michael@chromatic.com', - }, - }, - ], - interceptable: true, - retain: false, - status, - }; +export const getCalls = (finalStatus: CallStates) => { + const calls: Call[] = [ + { + id: 'story--id [3] within', + storyId: 'story--id', + cursor: 3, + path: [], + method: 'within', + args: [{ __element__: { localName: 'div', id: 'root' } }], + interceptable: false, + retain: false, + status: CallStates.DONE, + }, + { + id: 'story--id [4] findByText', + storyId: 'story--id', + cursor: 4, + path: [{ __callId__: 'story--id [3] within' }], + method: 'findByText', + args: ['Click'], + interceptable: true, + retain: false, + status: CallStates.DONE, + }, + { + id: 'story--id [5] click', + storyId: 'story--id', + cursor: 5, + path: ['userEvent'], + method: 'click', + args: [{ __element__: { localName: 'button', innerText: 'Click' } }], + interceptable: true, + retain: false, + status: CallStates.DONE, + }, + { + id: 'story--id [6] waitFor', + storyId: 'story--id', + cursor: 6, + path: [], + method: 'waitFor', + args: [{ __function__: { name: '' } }], + interceptable: true, + retain: false, + status: CallStates.DONE, + }, + { + id: 'story--id [6] waitFor [0] expect', + parentId: 'story--id [6] waitFor', + storyId: 'story--id', + cursor: 1, + path: [], + method: 'expect', + args: [{ __function__: { name: 'handleSubmit' } }], + interceptable: false, + retain: false, + status: CallStates.DONE, + }, + { + id: 'story--id [6] waitFor [1] stringMatching', + parentId: 'story--id [6] waitFor', + storyId: 'story--id', + cursor: 2, + path: ['expect'], + method: 'stringMatching', + args: [{ __regexp__: { flags: 'gi', source: '([A-Z])w+' } }], + interceptable: false, + retain: false, + status: CallStates.DONE, + }, + { + id: 'story--id [6] waitFor [2] toHaveBeenCalledWith', + parentId: 'story--id [6] waitFor', + storyId: 'story--id', + cursor: 3, + path: [{ __callId__: 'story--id [6] waitFor [0] expect' }], + method: 'toHaveBeenCalledWith', + args: [{ __callId__: 'story--id [6] waitFor [1] stringMatching', retain: false }], + interceptable: true, + retain: false, + status: CallStates.DONE, + }, + { + id: 'story--id [7] expect', + storyId: 'story--id', + cursor: 7, + path: [], + method: 'expect', + args: [{ __function__: { name: 'handleReset' } }], + interceptable: false, + retain: false, + status: CallStates.DONE, + }, + { + id: 'story--id [8] toHaveBeenCalled', + storyId: 'story--id', + cursor: 8, + path: [{ __callId__: 'story--id [7] expect' }, 'not'], + method: 'toHaveBeenCalled', + args: [], + interceptable: true, + retain: false, + status: finalStatus, + }, + ]; - const overrides = CallStates.ERROR - ? { exception: { name: 'Error', stack: '', message: 'Oops!', callId: defaultData.id } } - : {}; + if (finalStatus === CallStates.ERROR) { + calls[calls.length - 1].exception = { + name: 'Error', + stack: '', + message: 'Oops!', + callId: calls[calls.length - 1].id, + }; + } - return { ...defaultData, ...overrides }; + return calls; }; + +export const getInteractions = (finalStatus: CallStates) => + getCalls(finalStatus).filter((call) => call.interceptable);