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

Allow decorator API to accept HOC-style wrappers #1528

Closed
tmeasday opened this issue Jul 27, 2017 · 3 comments
Closed

Allow decorator API to accept HOC-style wrappers #1528

tmeasday opened this issue Jul 27, 2017 · 3 comments

Comments

@tmeasday
Copy link
Member

As per @shilman in https://gist.github.com/shilman/792dc25550daa9c2bf37238f4ef7a398:

We are moving to a "wrapper" style API for addons/decorators:

type story = (context) => renderable
type wrapper = story => story

Which can be used like:

.add('name', aWrapper(() => <story/>));

However, currently the decorator API takes something slightly different:

type decorator = (story, context) => renderable

It's conceptually the same, but different in practice.

Proposal:

  • In 3.3: Allow addDecorator to take both decorator and wrapper style functions.

  • In 3.4 (or sooner): deprecate the decorator style.

  • In 4.0: only allow wrappers.

Implementation.

Decorators are applied (in each app layer) in a line like this:

const fn = decorators.reduce(
  (decorated, decorator) => context => decorator(() => decorated(context), context),
  getStory
);

However, we can inspect the output of decorator(...) and see if it's

  1. A renderable (an React Element or an object in vue). (the old decorator case)
  2. Another function (the new story). (the new wrapper case).

In case 2, we can then just execute it.

So the new code looks like:

const fn = decorators.reduce(
  (decorated, decorator) => context => {
     if (decorator.length === 2) {
       // old case
       return decorator(() => decorated(context), context);
     }

     const newDecorated = decorator(() => decorated(context));

     // new case
     if (typeof newDecorated === function) {
       return newDecorated(context);
     }
     
     // old case, decorator doesn't consider context
     return newDecorated;
  },
  getStory
);

Phew! Confusing. But I think it works.

@shilman
Copy link
Member

shilman commented Jul 27, 2017

Yes, confusing as a transition path, but hopefully simple and uniform and powerful once we get through it! Thanks for putting this together!

@stale
Copy link

stale bot commented Oct 31, 2017

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. We do try to do some housekeeping every once in a while so inactive issues will get closed after 90 days. Thanks!

@stale stale bot added the inactive label Oct 31, 2017
@stale
Copy link

stale bot commented Nov 2, 2017

Hey there, it's me again! I am going to help our maintainers close this issue so they can focus on development efforts instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!

@stale stale bot closed this as completed Nov 2, 2017
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

2 participants