Skip to content

Commit

Permalink
Merge pull request #9868 from storybookjs/client-api-hot-refresh
Browse files Browse the repository at this point in the history 
Core: Add skip dispose option to ClientApi
  • Loading branch information
@tmeasday @shilman
tmeasday authored and shilman committed Aug 28, 2020
1 parent e5cff07 commit 4331b85
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 35 deletions.
95 changes: 62 additions & 33 deletions lib/client-api/src/client_api.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@ import ClientApi from './client_api';
import ConfigApi from './config_api';
import StoryStore from './story_store';

const getContext = (() => decorateStory => {
const getContext = ({
decorateStory = undefined,
noStoryModuleAddMethodHotDispose = false,
} = {}) => {
const channel = mockChannel();
addons.setChannel(channel);
const storyStore = new StoryStore({ channel });
const clientApi = new ClientApi({ storyStore, decorateStory });
const clientApi = new ClientApi({ storyStore, decorateStory, noStoryModuleAddMethodHotDispose });
const { clearDecorators } = clientApi;
const configApi = new ConfigApi({ clearDecorators, storyStore, channel, clientApi });

Expand All @@ -19,21 +22,21 @@ const getContext = (() => decorateStory => {
channel,
clientApi,
};
})();
};

jest.mock('@storybook/client-logger', () => ({
logger: { warn: jest.fn(), log: jest.fn() },
}));

describe('preview.client_api', () => {
afterEach(() => {
const { clientApi } = getContext(undefined);
const { clientApi } = getContext();
clientApi.clearDecorators();
clientApi.clearParameters();
});
describe('setAddon', () => {
it('should register addons', () => {
const { clientApi } = getContext(undefined);
const { clientApi } = getContext();
let data;

clientApi.setAddon({
Expand All @@ -47,7 +50,7 @@ describe('preview.client_api', () => {
});

it('should not remove previous addons', () => {
const { clientApi } = getContext(undefined);
const { clientApi } = getContext();
const data = [];

clientApi.setAddon({
Expand All @@ -70,7 +73,7 @@ describe('preview.client_api', () => {
});

it('should call with the clientApi context', () => {
const { clientApi } = getContext(undefined);
const { clientApi } = getContext();
let data;

clientApi.setAddon({
Expand All @@ -84,7 +87,7 @@ describe('preview.client_api', () => {
});

it('should be able to access addons added previously', () => {
const { clientApi } = getContext(undefined);
const { clientApi } = getContext();
let data;

clientApi.setAddon({
Expand All @@ -104,7 +107,7 @@ describe('preview.client_api', () => {
});

it('should be able to access the current kind', () => {
const { clientApi } = getContext(undefined);
const { clientApi } = getContext();
const kind = 'dfdwf3e3';
let data;

Expand All @@ -121,7 +124,7 @@ describe('preview.client_api', () => {

describe('addParameters', () => {
it('should add parameters', () => {
const { clientApi, storyStore } = getContext(undefined);
const { clientApi, storyStore } = getContext();
const { storiesOf } = clientApi;

clientApi.addParameters({ a: 1 });
Expand All @@ -135,7 +138,7 @@ describe('preview.client_api', () => {
});

it('should merge options', () => {
const { clientApi, storyStore } = getContext(undefined);
const { clientApi, storyStore } = getContext();
const { storiesOf } = clientApi;

clientApi.addParameters({ options: { a: '1' } });
Expand All @@ -151,7 +154,7 @@ describe('preview.client_api', () => {
});

it('should override specific properties in options', () => {
const { clientApi, storyStore } = getContext(undefined);
const { clientApi, storyStore } = getContext();
const { storiesOf } = clientApi;

clientApi.addParameters({ backgrounds: ['value'], options: { a: '1', b: '3' } });
Expand All @@ -169,7 +172,7 @@ describe('preview.client_api', () => {
});

it('should replace top level properties and override specific properties in options', () => {
const { clientApi, storyStore } = getContext(undefined);
const { clientApi, storyStore } = getContext();
const { storiesOf } = clientApi;

clientApi.addParameters({ backgrounds: ['value'], options: { a: '1', b: '3' } });
Expand All @@ -187,7 +190,7 @@ describe('preview.client_api', () => {
});

it('should deep merge in options', () => {
const { clientApi, storyStore } = getContext(undefined);
const { clientApi, storyStore } = getContext();
const { storiesOf } = clientApi;

clientApi.addParameters({ options: { a: '1', b: '2', theming: { c: '3' } } });
Expand All @@ -208,7 +211,7 @@ describe('preview.client_api', () => {
const {
clientApi: { storiesOf },
storyStore,
} = getContext(undefined);
} = getContext();

storiesOf('kind', module)
.addDecorator(fn => `aa-${fn()}`)
Expand All @@ -221,7 +224,7 @@ describe('preview.client_api', () => {
const {
clientApi: { addDecorator, storiesOf },
storyStore,
} = getContext(undefined);
} = getContext();

addDecorator(fn => `bb-${fn()}`);

Expand All @@ -235,7 +238,7 @@ describe('preview.client_api', () => {
const {
clientApi: { addDecorator, storiesOf },
storyStore,
} = getContext(undefined);
} = getContext();

addDecorator(fn => `aa-${fn()}`);

Expand All @@ -250,7 +253,7 @@ describe('preview.client_api', () => {
const {
clientApi: { storiesOf },
storyStore,
} = getContext(undefined);
} = getContext();

storiesOf('kind', module)
.addDecorator(fn => `aa-${fn()}`)
Expand All @@ -264,7 +267,7 @@ describe('preview.client_api', () => {
const {
clientApi: { storiesOf },
storyStore,
} = getContext(undefined);
} = getContext();

storiesOf('kind', module)
.addDecorator((fn, { kind, name }) => `${kind}-${name}-${fn()}`)
Expand All @@ -277,7 +280,7 @@ describe('preview.client_api', () => {

describe('clearDecorators', () => {
it('should remove all global decorators', () => {
const { clientApi, storyStore } = getContext(undefined);
const { clientApi, storyStore } = getContext();
const { storiesOf } = clientApi;

clientApi.addDecorator(() => 'foo');
Expand All @@ -294,7 +297,7 @@ describe('preview.client_api', () => {
it('should transform the storybook to an array with filenames', () => {
const {
clientApi: { getStorybook, storiesOf },
} = getContext(undefined);
} = getContext();

let book;

Expand Down Expand Up @@ -347,7 +350,7 @@ describe('preview.client_api', () => {
it('returns values set via parameters', () => {
const {
clientApi: { getSeparators, storiesOf, addParameters },
} = getContext(undefined);
} = getContext();

const options = { hierarchySeparator: /a/, hierarchyRootSeparator: 'b' };
addParameters({ options });
Expand All @@ -358,7 +361,7 @@ describe('preview.client_api', () => {
it('returns old defaults if kind uses old separators', () => {
const {
clientApi: { getSeparators, storiesOf },
} = getContext(undefined);
} = getContext();

storiesOf('kind|1', module).add('name 1', () => '1');
expect(getSeparators()).toEqual({
Expand All @@ -370,7 +373,7 @@ describe('preview.client_api', () => {
it('returns new values if showRoots is set', () => {
const {
clientApi: { getSeparators, storiesOf, addParameters },
} = getContext(undefined);
} = getContext();
addParameters({ options: { showRoots: false } });

storiesOf('kind|1', module).add('name 1', () => '1');
Expand All @@ -380,7 +383,7 @@ describe('preview.client_api', () => {
it('returns new values if kind does not use old separators', () => {
const {
clientApi: { getSeparators, storiesOf },
} = getContext(undefined);
} = getContext();

storiesOf('kind/1', module).add('name 1', () => '1');
expect(getSeparators()).toEqual({ hierarchySeparator: '/' });
Expand All @@ -390,7 +393,7 @@ describe('preview.client_api', () => {
it('reads filename from module', () => {
const {
clientApi: { getStorybook, storiesOf },
} = getContext(undefined);
} = getContext();

const fn = jest.fn();
storiesOf('kind', { id: 'foo.js' }).add('name', fn);
Expand All @@ -414,7 +417,7 @@ describe('preview.client_api', () => {
it('should stringify ids from module', () => {
const {
clientApi: { getStorybook, storiesOf },
} = getContext(undefined);
} = getContext();

const fn = jest.fn();
storiesOf('kind', { id: 1211 }).add('name', fn);
Expand Down Expand Up @@ -455,7 +458,7 @@ describe('preview.client_api', () => {
const {
storyStore,
clientApi: { storiesOf },
} = getContext(undefined);
} = getContext();
const module = new MockModule();

expect(storyStore.getRevision()).toEqual(0);
Expand All @@ -470,7 +473,7 @@ describe('preview.client_api', () => {
it('should replace a kind when the module reloads', () => {
const {
clientApi: { storiesOf, getStorybook },
} = getContext(undefined);
} = getContext();
const module = new MockModule();

const stories = [jest.fn(), jest.fn()];
Expand Down Expand Up @@ -513,7 +516,7 @@ describe('preview.client_api', () => {
const {
clientApi: { storiesOf, getStorybook },
channel,
} = getContext(undefined);
} = getContext();
const module0 = new MockModule();
const module1 = new MockModule();
const module2 = new MockModule();
Expand Down Expand Up @@ -548,14 +551,40 @@ describe('preview.client_api', () => {
expect(Object.values(args.stories).map(v => v.kind)).toEqual(['kind0', 'kind1', 'kind2']);
expect(getStorybook().map(story => story.kind)).toEqual(['kind1', 'kind2']);
});

it('should call `module.hot.dispose` inside add and soriesOf by default', () => {
const module = new MockModule();
module.hot.dispose = jest.fn();

const {
clientApi: { storiesOf, getStorybook },
} = getContext();

storiesOf('kind', module).add('story', jest.fn());

expect(module.hot.dispose.mock.calls.length).toEqual(2);
});

it('should not call `module.hot.dispose` inside add when noStoryModuleAddMethodHotDispose is true', () => {
const module = new MockModule();
module.hot.dispose = jest.fn();

const {
clientApi: { storiesOf, getStorybook },
} = getContext({ noStoryModuleAddMethodHotDispose: true });

storiesOf('kind', module).add('story', jest.fn());

expect(module.hot.dispose.mock.calls.length).toEqual(1);
});
});

describe('parameters', () => {
it('collects parameters across different modalities', () => {
const {
storyStore,
clientApi: { storiesOf, addParameters },
} = getContext(undefined);
} = getContext();

addParameters({ a: 'global', b: 'global', c: 'global' });

Expand All @@ -578,7 +607,7 @@ describe('preview.client_api', () => {
const {
storyStore,
clientApi: { storiesOf, addParameters },
} = getContext(undefined);
} = getContext();

addParameters({
addon1: 'global string value',
Expand Down Expand Up @@ -638,7 +667,7 @@ describe('preview.client_api', () => {

const {
clientApi: { storiesOf, getStorybook },
} = getContext(undefined);
} = getContext();

expect(getStorybook()).toEqual([]);

Expand Down
14 changes: 12 additions & 2 deletions lib/client-api/src/client_api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,19 @@ export default class ClientApi {

private _decorateStory: (storyFn: StoryFn, decorators: DecoratorFunction[]) => any;

constructor({ storyStore, decorateStory = defaultDecorateStory }: ClientApiParams) {
// React Native Fast refresh doesn't allow multiple dispose calls
private _noStoryModuleAddMethodHotDispose: boolean;

constructor({
storyStore,
decorateStory = defaultDecorateStory,
noStoryModuleAddMethodHotDispose,
}: ClientApiParams) {
this._storyStore = storyStore;
this._addons = {};

this._noStoryModuleAddMethodHotDispose = noStoryModuleAddMethodHotDispose || false;

this._decorateStory = decorateStory;

if (!storyStore) {
Expand Down Expand Up @@ -231,7 +240,8 @@ export default class ClientApi {
if (typeof storyName !== 'string') {
throw new Error(`Invalid or missing storyName provided for a "${kind}" story.`);
}
if (m && m.hot && m.hot.dispose) {

if (!this._noStoryModuleAddMethodHotDispose && m && m.hot && m.hot.dispose) {
m.hot.dispose(() => {
const { _storyStore } = this;
_storyStore.remove(id);
Expand Down
1 change: 1 addition & 0 deletions lib/client-api/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface StoreData {
export interface ClientApiParams {
storyStore: StoryStore;
decorateStory?: (storyFn: any, decorators: any) => any;
noStoryModuleAddMethodHotDispose?: boolean;
}

export type ClientApiReturnFn<StoryFnReturnType> = (...args: any[]) => StoryApi<StoryFnReturnType>;
Expand Down

0 comments on commit 4331b85

Please sign in to comment.