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

Addon-interactions: Low level instrumentation API #16667

Closed
cowboyd opened this issue Nov 11, 2021 · 13 comments
Closed

Addon-interactions: Low level instrumentation API #16667

cowboyd opened this issue Nov 11, 2021 · 13 comments

Comments

@cowboyd
Copy link

cowboyd commented Nov 11, 2021

Is your feature request related to a problem? Please describe
We're attempting an integration of interactors with storybook. The good news is that we were up and running in just a few minutes. You can see the results here thefrontside/interactors#126

However, we were only able to get this working by getting a reference to the internal instrumentation's track() method and invoking it manually.

track(method: string, fn: Function, args: any[], options: Options) {
const storyId: StoryId =
args?.[0]?.__storyId__ || global.window.__STORYBOOK_PREVIEW__?.urlStore?.selection?.storyId;
const index = this.getState(storyId).cursor;
this.setState(storyId, { cursor: index + 1 });
const id = `${storyId} [${index}] ${method}`;
const { path = [], intercept = false, retain = false } = options;
const interceptable = typeof intercept === 'function' ? intercept(method, path) : intercept;
const call: Call = { id, path, method, storyId, 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 }] });
}

This was necessary because unlike testing library, interactors are page objects for components, and their actions live on the objects themselves which are created at runtime, rather than available as static module methods.

The good news is that it actually made the integration less work in the sense that interactors already have a native instrumentation mechanism, and so all we needed to do was to integrate it with the underlying storybook instrumentation method.

Describe the solution you'd like
Would love to see an officially supported instrumentation API that sits underneath, and is used by, the instrumenter() API. Not only would it allow non-static libraries like interactors to hook directly into the interaction api, but it would also mean that any other library x that made use of this API could be imported directly into story book rather than having to publish a separate storybook-instrumented-x for its users.

This API would need to:

  1. to support passing all of the information that will be rendered onto the UI
  2. be forwards compatible with any grouping API that arranged interactions into some sort of expandable tree.
  3. be forwards compatible with any timing api that specified categorical timing (slow, fast, no delay, etc...)

Maybe something like this?

import { onPlay } from '@storybook/instrumentation';

onPlay(async (act: ActionFn, signal: AbortSignal) => {
  await act('first action', async () => {/* actually do the thing */});
  await act('second action', async () => { /* do something else * });
});

Where ActionFn is maybe so:

interface ActionFn {
  (description: string, implementation: () => Promise<unknown>);
}

Are you able to assist to bring the feature to reality?
Definitely!

Additional context
Interactors, page objects for components.

@andrewluetgers
Copy link

I think the Jest lifecycle is a familiar and solid pattern to look at. https://noriste.github.io/reactjsday-2019-testing-course/book/jest-101/jest-lifecycle.html

@andrewluetgers
Copy link

Honestly feel like play functions should be interchangeable with Jest tests but thats just me.

@cowboyd
Copy link
Author

cowboyd commented Nov 15, 2021

I think the Jest lifecycle is a familiar and solid pattern to look at. https://noriste.github.io/reactjsday-2019-testing-course/book/jest-101/jest-lifecycle.html

@andrewluetgers Are you imagining something similar but using the beforeEach name? E.g.

import { beforeEach } from '@storybook/instrumentation';

beforeEach(async (act: ActionFn, signal: AbortSignal) => {
  // setup integration here.
});

@cowboyd
Copy link
Author

cowboyd commented Nov 15, 2021

If we can agree on a general direction to take with the API, we'd be happy to take a shot at spiking an implementation.

@shilman shilman added linear and removed linear labels Nov 15, 2021
@andrewluetgers
Copy link

andrewluetgers commented Nov 15, 2021

@cowboyd yeah but now that I think of it there is no concept of a suite of play functions equivalent to a jest suite. I suppose beforeEach makes sense for a global rule but the convention for storybook to apply global rules like that in the preview.js.

@shilman shilman changed the title Low level instrumentation API Addon-interactions: Low level instrumentation API Nov 15, 2021
@wKich
Copy link
Member

wKich commented Nov 16, 2021

I think maybe it should not be as a part of instrument API, but instead create new API that allows to define custom steps by passing there special structure which describes js statement in ast-like format.

@cowboyd
Copy link
Author

cowboyd commented Nov 17, 2021

I think maybe it should not be as a part of instrument API, but instead create new API that allows to define custom steps by passing there special structure which describes js statement in ast-like format.

@wKich Sounds coo, although I'm not sure what it would look like in practice. Can you post an example of what you're thinking?

@wKich
Copy link
Member

wKich commented Nov 17, 2021

The steps API could look like this:

// Menu('Actions', { disabled: or(true, false) }).find(Item('First')).click()
defineStep(
  fn,
  object(
    call('Menu', ['Actions', { disabled: call('or', [true, false]) }]),
    call('find', [
      call('Item', ['First'])
    ]),
    call('click')
  )
)

@cowboyd
Copy link
Author

cowboyd commented Nov 17, 2021

How would the full integration work though? I'm not quite seeing it. Can you show what the full integration between interaction and storybook would look like?

@cowboyd
Copy link
Author

cowboyd commented Nov 19, 2021

The steps API could look like this:

To clarify, one of the things that we're realizing is that when we define a step in storybook, we'll need some way of integrating with the highlighter. I think that's what @wKich as getting at by effectively defining an AST of some sort that the debugger could use to render appropriately. Rather than require some one-size-fits all syntax tree, what if we just passed the highlighter to the steps?

import { onPlay } from '@storybook/interactions';

onPlay(async (step: StepFn, signal: AbortSignal, h: Highlighter) => {
  // highlight with bundled js highlighter
  step(h("Do(some(thing()))"), async () => { /* do the thing */ });
  // highlight with bundled text highlighter
  step(h("do something else", type: 'text/plain' }), async => { /* do the thing */ });
  // use a custom highlighter function
  step(h("(use (some (lispy thing)))", type: txt => coderay(text, { type: 'clojurescript' }))
});

@ghengeveld
Copy link
Member

ghengeveld commented Nov 19, 2021

Can you clarify what this highlighter does?

Regarding the Jest API suggestion (e.g. beforeEach), we did talk about this but concluded that the Jest scoping patterns (describe, it) simply map onto Storybook's existing story hierarchy, where a story is basically a describe block. A beforeEach can be achieved using a loader, decorator or custom render function, and an afterEach would just be part of the play function. It's just different concepts that can achieve the same things. As such, we don't really have a need to support those extra APIs.

What we do have in mind (and will add in 6.5) is a step API, which is like an it block in Jest, but optional:

play: async () => {
  await step("Select product and quantity", async () => {
    await userEvent.select(...)
    await userEvent.type(...)
  })
  await step("Select shipping method", () => userEvent.click(...))
  await step("Checkout", async () => {
    await userEvent.type(...)
    await userEvent.type(...)
    await userEvent.click(...)
  })
}

Effectively these are just "labeled groups". They don't really do anything special beyond making the interaction log more descriptive.

@wKich
Copy link
Member

wKich commented Nov 22, 2021

@ghengeveld the highlighter is the same highlighter that is used in Storybook to highlight step's code.

I like your step suggestion, we can wrap actions and call steps internally, so the user doesn't need to write them by using intetractors. But there will be a problem if the user wraps interactors in the step function explicitly and we wrap interaction in step too. Will storybook be able to handle this? Or do we need to think about another approach? We are able to help you to implement an API that fits interactions and your needs. Maybe step's API should provide information about is execution in step context or not. So we can skip wrapping on our side if user already wrapped interactors

Does step allow to pass highlighted string or source code string as a first argument?

@yannbf
Copy link
Member

yannbf commented Dec 1, 2022

Hey @cowboyd @wKich sorry for the late notice, but the step function has been implemented and is usable as of Storybook 7.0.

export const MyStory = {
  play: async (context) => {
    await step("Select product and quantity", async () => {
      await userEvent.select(...)
      await userEvent.type(...)
    })
    // you can reuse other story's play function as a step, the story context will be passed down to that callback
    await step("Select shipping method", SelectShipping.play)
    await step("Checkout", async () => {
      await userEvent.type(...)
      await userEvent.type(...)
      await userEvent.click(...)
    })
  }
}

The step can be augmented by addons which expose a stepRunner as a preset. This is how the addon-interactions augments it:
https://github.com/storybookjs/storybook/blob/next/code/addons/interactions/src/preview.ts#L60

This is a code example of actual usage:
https://github.com/storybookjs/storybook/blob/next/code/addons/interactions/template/stories/basics.stories.ts#L44

This is where the step runners are composed:
https://github.com/storybookjs/storybook/blob/next/code/lib/preview-api/src/modules/client-api/ClientApi.ts#L178

I'll be closing this issue, but feel free to experiment and check if this is good for your scenario, and open the issue again it shouldn't have been closed.

@yannbf yannbf closed this as completed Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants