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

Implement preview hooks #6916

Merged
merged 23 commits into from
Jul 10, 2019
Merged

Implement preview hooks #6916

merged 23 commits into from
Jul 10, 2019

Conversation

Hypnosphi
Copy link
Member

@Hypnosphi Hypnosphi commented May 30, 2019

This PR adds a framework-agnostic implementation of API similar to React Hooks

Here, the story render lifecycle is an analogue of React render lifecycle, while decorators and story functions are "components".
The main purpose is simplifying addon development. E.g. useEffect serves as a replacement for REGISTER_SUBSCRIPTION event.

@vercel
Copy link

vercel bot commented May 30, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-preview-hooks.storybook.now.sh

@shilman
Copy link
Member

shilman commented May 31, 2019

Amazing @Hypnosphi

# Conflicts:
#	addons/actions/package.json
#	addons/events/package.json
#	examples/html-kitchen-sink/package.json
@ndelangen
Copy link
Member

Do you think we can create hooks like: useContext, useParameter & useChannel?
I think those will be the most impactful ones.

The code needs documentation. I think this low level code is easy to forget how exactly it works. It feels to me, code comments (maybe jsdoc?) in the code would be great.
Possibly splitting all hooks into their own file/module?

@Hypnosphi
Copy link
Member Author

Hypnosphi commented Jun 8, 2019

By context, do you mean the story context? I think we can. I would prefer some name like useStoryContext to indicate that it has nothing to do with React.useContext while the other hooks do more or less the same things as their React counterparts

What should useChannel do?

@Hypnosphi
Copy link
Member Author

@ndelangen I will mostly be far from computer in the following days so feel free to add whatever makes sense to you

@ndelangen
Copy link
Member

What should useChannel do?

useChannel

import { STORY_CHANGED } from '@storybook/core-events';
export const Panel = () => {
  const emit = useChannel({
    STORY_CHANGED: (...args) => console.log(...args),
  });

  return (
    <button onClick={() => emit('my-event-type', { some: 'data' })}>
      clicking this will emit an event
    </button>
  );
}

Allows for both setting subscriptions to events and getting the emitter for emitting custom event unto the channel.

The messages can be listened for on both the iframe and the manager side.

@ndelangen
Copy link
Member

ndelangen commented Jun 12, 2019

Here's how it's implemented in the lib/api:

export const useChannel = (eventMap: EventMap) => {
  const api = useStorybookApi();
  useEffect(() => {
    Object.entries(eventMap).forEach(([type, listener]) => api.on(type, listener));
    return () => {
      Object.entries(eventMap).forEach(([type, listener]) => api.off(type, listener));
    };
  });

  return api.emit;
};

@Hypnosphi
Copy link
Member Author

@shilman are there any blockers left here?

# Conflicts:
#	addons/actions/package.json
#	addons/events/package.json
#	addons/events/src/index.ts
#	addons/ondevice-notes/package.json
#	dev-kits/addon-roundtrip/package.json
#	examples/dev-kits/package.json
#	examples/html-kitchen-sink/package.json
#	lib/client-api/package.json
#	lib/client-api/src/client_api.ts
#	lib/client-api/src/typings.d.ts
#	lib/client-api/tsconfig.json
#	lib/ui/src/index.js
#	yarn.lock
@ndelangen
Copy link
Member

I think not, I'd love to merge this and start using it in devkits & addons!

I'm pushing for this to make it into 5.2

@@ -1,14 +1,9 @@
// Based on http://backbonejs.org/docs/backbone.html#section-164
import { document, Element } from 'global';
import { isEqual } from 'lodash';
Copy link
Member

@ndelangen ndelangen Jul 10, 2019

Choose a reason for hiding this comment

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

nice! This should shave off quite some bundlesize


const root = document && document.getElementById('root');

export const createDecorator = () => (options: any) => (storyFn: () => any) => {
Copy link
Member

Choose a reason for hiding this comment

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

@ndelangen @Hypnosphi StoryContext?

@shilman
Copy link
Member

shilman commented Jul 10, 2019

@ndelangen @Hypnosphi I'd like to see examples & documentation before we release, but aside from the comment (question) above, I'm fine to merge this.

@ndelangen
Copy link
Member

@shilman I promise that I'll document this more during the beta.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants