Merge pull request #3559 from storybooks/tmeasday/refactor-transitional-decorator

Refactor transitional decorator from addon-notes

addons/notes/src/__tests__/index.js

@@ -1,7 +1,7 @@
 import addons from '@storybook/addons';
 import { withNotes } from '..';
 
-jest.mock('@storybook/addons');
+addons.getChannel = jest.fn();
 
 describe('Storybook Addon Notes', () => {
   it('should inject text from `notes` parameter', () => {
@@ -16,6 +16,18 @@ describe('Storybook Addon Notes', () => {
     expect(getStory).toHaveBeenCalledWith(context);
   });
 
+  it('should NOT inject text if no `notes` parameter', () => {
+    const channel = { emit: jest.fn() };
+    addons.getChannel.mockReturnValue(channel);
+
+    const getStory = jest.fn();
+    const context = {};
+
+    withNotes(getStory, context);
+    expect(channel.emit).not.toHaveBeenCalled();
+    expect(getStory).toHaveBeenCalledWith(context);
+  });
+
   it('should inject markdown from `notes.markdown` parameter', () => {
     const channel = { emit: jest.fn() };
     addons.getChannel.mockReturnValue(channel);

addons/notes/src/index.js

@@ -1,4 +1,4 @@
-import addons from '@storybook/addons';
+import addons, { makeDecorator } from '@storybook/addons';
 import marked from 'marked';
 
 function renderMarkdown(text, options) {
@@ -6,43 +6,30 @@ function renderMarkdown(text, options) {
   return marked(text);
 }
 
-const decorator = options => {
-  const channel = addons.getChannel();
-  return (getStory, context) => {
-    const {
-      parameters: { notes },
-    } = context;
-    const storyOptions = notes || options;
+export const withNotes = makeDecorator({
+  name: 'withNotes',
+  parameterName: 'notes',
+  skipIfNoParametersOrOptions: true,
+  wrapper: (getStory, context, { options, parameters }) => {
+    const channel = addons.getChannel();
 
-    if (storyOptions) {
-      const { text, markdown, markdownOptions } =
-        typeof storyOptions === 'string' ? { text: storyOptions } : storyOptions;
+    const storyOptions = parameters || options;
 
-      if (!text && !markdown) {
-        throw new Error('You must set of one of `text` or `markdown` on the `notes` parameter');
-      }
+    const { text, markdown, markdownOptions } =
+      typeof storyOptions === 'string' ? { text: storyOptions } : storyOptions;
 
-      channel.emit('storybook/notes/add_notes', text || renderMarkdown(markdown, markdownOptions));
+    if (!text && !markdown) {
+      throw new Error('You must set of one of `text` or `markdown` on the `notes` parameter');
     }
 
-    return getStory(context);
-  };
-};
+    channel.emit('storybook/notes/add_notes', text || renderMarkdown(markdown, markdownOptions));
 
-const hoc = options => story => context => decorator(options)(story, context);
+    return getStory(context);
+  },
+});
 
 export const withMarkdownNotes = (text, options) =>
-  hoc({
+  withNotes({
     markdown: text,
     markdownOptions: options,
   });
-
-export const withNotes = (...args) => {
-  // Used without options as .addDecorator(withNotes)
-  if (typeof args[0] === 'function') {
-    return decorator()(...args);
-  }
-
-  // Input are options, ala .add('name', withNotes('note')(() => <Story/>))
-  return hoc(args[0]);
-};

lib/addons/package.json

@@ -21,6 +21,7 @@
   },
   "dependencies": {
     "@storybook/channels": "4.0.0-alpha.6",
-    "global": "^4.3.2"
+    "global": "^4.3.2",
+    "util-deprecate": "^1.0.2"
   }
 }

lib/addons/src/index.js

@@ -2,6 +2,7 @@
 import global from 'global';
 
 export mockChannel from './storybook-channel-mock';
+export { makeDecorator } from './make-decorator';
 
 export class AddonStore {
   constructor() {

lib/addons/src/make-decorator.js

@@ -0,0 +1,51 @@
+import deprecate from 'util-deprecate';
+
+// Create a decorator that can be used both in the (deprecated) old "hoc" style:
+//   .add('story', decorator(options)(() => <Story />));
+//
+// And in the new, "parameterized" style:
+//   .addDecorator(decorator)
+//   .add('story', () => <Story />, { name: { parameters } });
+//
+// *And* in the older, but not deprecated, "pass options to decorator" style:
+//  .addDecorator(decorator(options))
+
+export const makeDecorator = ({
+  name,
+  parameterName,
+  wrapper,
+  skipIfNoParametersOrOptions = false,
+}) => {
+  const decorator = options => (getStory, context) => {
+    const parameters = context.parameters && context.parameters[parameterName];
+
+    if (skipIfNoParametersOrOptions && !options && !parameters) {
+      return getStory(context);
+    }
+    return wrapper(getStory, context, {
+      options,
+      parameters,
+    });
+  };
+
+  return (...args) => {
+    // Used without options as .addDecorator(decorator)
+    if (typeof args[0] === 'function') {
+      return decorator()(...args);
+    }
+
+    return (...innerArgs) => {
+      // Used as [.]addDecorator(decorator(options))
+      if (innerArgs.length > 1) {
+        return decorator(...args)(...innerArgs);
+      }
+
+      // Used to wrap a story directly .add('story', decorator(options)(() => <Story />))
+      //   This is now deprecated:
+      return deprecate(
+        context => decorator(...args)(innerArgs[0], context),
+        `Passing stories directly into ${name}() is deprecated, instead use addDecorator(${name}) and pass options with the '${parameterName}' parameter`
+      );
+    };
+  };
+};

lib/addons/src/make-decorator.test.js

@@ -0,0 +1,106 @@
+import deprecate from 'util-deprecate';
+import { makeDecorator } from './make-decorator';
+import { defaultDecorateStory } from '../../../lib/core/src/client/preview/client_api';
+
+jest.mock('util-deprecate');
+let deprecatedFns = [];
+deprecate.mockImplementation((fn, warning) => {
+  const deprecatedFn = jest.fn(fn);
+  deprecatedFns.push({
+    deprecatedFn,
+    warning,
+  });
+  return deprecatedFn;
+});
+
+describe('makeDecorator', () => {
+  it('returns a decorator that passes parameters on the parameters argument', () => {
+    const wrapper = jest.fn();
+    const decorator = makeDecorator({ wrapper, name: 'test', parameterName: 'test' });
+    const story = jest.fn();
+    const decoratedStory = defaultDecorateStory(story, [decorator]);
+
+    const context = { parameters: { test: 'test-val' } };
+    decoratedStory(context);
+
+    expect(wrapper).toHaveBeenCalledWith(expect.any(Function), context, { parameters: 'test-val' });
+  });
+
+  it('passes options added at decoration time', () => {
+    const wrapper = jest.fn();
+    const decorator = makeDecorator({ wrapper, name: 'test', parameterName: 'test' });
+    const story = jest.fn();
+    const options = 'test-val';
+    const decoratedStory = defaultDecorateStory(story, [decorator(options)]);
+
+    const context = {};
+    decoratedStory(context);
+
+    expect(wrapper).toHaveBeenCalledWith(expect.any(Function), context, { options: 'test-val' });
+  });
+
+  it('passes both options *and* parameters at the same time', () => {
+    const wrapper = jest.fn();
+    const decorator = makeDecorator({ wrapper, name: 'test', parameterName: 'test' });
+    const story = jest.fn();
+    const options = 'test-val';
+    const decoratedStory = defaultDecorateStory(story, [decorator(options)]);
+
+    const context = { parameters: { test: 'test-val' } };
+    decoratedStory(context);
+
+    expect(wrapper).toHaveBeenCalledWith(expect.any(Function), context, {
+      options: 'test-val',
+      parameters: 'test-val',
+    });
+  });
+
+  it('passes nothing if neither are supplied', () => {
+    const wrapper = jest.fn();
+    const decorator = makeDecorator({ wrapper, name: 'test', parameterName: 'test' });
+    const story = jest.fn();
+    const decoratedStory = defaultDecorateStory(story, [decorator]);
+
+    const context = {};
+    decoratedStory(context);
+
+    expect(wrapper).toHaveBeenCalledWith(expect.any(Function), context, {});
+  });
+
+  it('calls the story directly if  neither are supplied and skipIfNoParametersOrOptions is true', () => {
+    const wrapper = jest.fn();
+    const decorator = makeDecorator({
+      wrapper,
+      name: 'test',
+      parameterName: 'test',
+      skipIfNoParametersOrOptions: true,
+    });
+    const story = jest.fn();
+    const decoratedStory = defaultDecorateStory(story, [decorator]);
+
+    const context = {};
+    decoratedStory(context);
+
+    expect(wrapper).not.toHaveBeenCalled();
+    expect(story).toHaveBeenCalled();
+  });
+
+  it('passes options added at story time, but with a deprecation warning', () => {
+    deprecatedFns = [];
+    const wrapper = jest.fn();
+    const decorator = makeDecorator({ wrapper, name: 'test', parameterName: 'test' });
+    const options = 'test-val';
+    const story = jest.fn();
+    const decoratedStory = decorator(options)(story);
+    expect(deprecatedFns).toHaveLength(1);
+    expect(deprecatedFns[0].warning).toMatch('addDecorator(test)');
+
+    const context = {};
+    decoratedStory(context);
+
+    expect(wrapper).toHaveBeenCalledWith(expect.any(Function), context, {
+      options: 'test-val',
+    });
+    expect(deprecatedFns[0].deprecatedFn).toHaveBeenCalled();
+  });
+});

lib/core/src/client/preview/client_api.js

@@ -4,7 +4,7 @@ import { logger } from '@storybook/client-logger';
 
 import StoryStore from './story_store';
 
-const defaultDecorateStory = (getStory, decorators) =>
+export const defaultDecorateStory = (getStory, decorators) =>
   decorators.reduce(
     (decorated, decorator) => context => decorator(() => decorated(context), context),
     getStory