From 4331b8546379812ab74c079765e9628e47df748b Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Thu, 20 Feb 2020 09:20:38 +1100 Subject: [PATCH] Merge pull request #9868 from storybookjs/client-api-hot-refresh Core: Add skip dispose option to ClientApi --- lib/client-api/src/client_api.test.ts | 95 +++++++++++++++++---------- lib/client-api/src/client_api.ts | 14 +++- lib/client-api/src/types.ts | 1 + 3 files changed, 75 insertions(+), 35 deletions(-) diff --git a/lib/client-api/src/client_api.test.ts b/lib/client-api/src/client_api.test.ts index 4ebf4fd45d37..43bc117249f5 100644 --- a/lib/client-api/src/client_api.test.ts +++ b/lib/client-api/src/client_api.test.ts @@ -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 }); @@ -19,7 +22,7 @@ const getContext = (() => decorateStory => { channel, clientApi, }; -})(); +}; jest.mock('@storybook/client-logger', () => ({ logger: { warn: jest.fn(), log: jest.fn() }, @@ -27,13 +30,13 @@ jest.mock('@storybook/client-logger', () => ({ 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({ @@ -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({ @@ -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({ @@ -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({ @@ -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; @@ -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 }); @@ -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' } }); @@ -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' } }); @@ -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' } }); @@ -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' } } }); @@ -208,7 +211,7 @@ describe('preview.client_api', () => { const { clientApi: { storiesOf }, storyStore, - } = getContext(undefined); + } = getContext(); storiesOf('kind', module) .addDecorator(fn => `aa-${fn()}`) @@ -221,7 +224,7 @@ describe('preview.client_api', () => { const { clientApi: { addDecorator, storiesOf }, storyStore, - } = getContext(undefined); + } = getContext(); addDecorator(fn => `bb-${fn()}`); @@ -235,7 +238,7 @@ describe('preview.client_api', () => { const { clientApi: { addDecorator, storiesOf }, storyStore, - } = getContext(undefined); + } = getContext(); addDecorator(fn => `aa-${fn()}`); @@ -250,7 +253,7 @@ describe('preview.client_api', () => { const { clientApi: { storiesOf }, storyStore, - } = getContext(undefined); + } = getContext(); storiesOf('kind', module) .addDecorator(fn => `aa-${fn()}`) @@ -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()}`) @@ -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'); @@ -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; @@ -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 }); @@ -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({ @@ -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'); @@ -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: '/' }); @@ -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); @@ -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); @@ -455,7 +458,7 @@ describe('preview.client_api', () => { const { storyStore, clientApi: { storiesOf }, - } = getContext(undefined); + } = getContext(); const module = new MockModule(); expect(storyStore.getRevision()).toEqual(0); @@ -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()]; @@ -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(); @@ -548,6 +551,32 @@ 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', () => { @@ -555,7 +584,7 @@ describe('preview.client_api', () => { const { storyStore, clientApi: { storiesOf, addParameters }, - } = getContext(undefined); + } = getContext(); addParameters({ a: 'global', b: 'global', c: 'global' }); @@ -578,7 +607,7 @@ describe('preview.client_api', () => { const { storyStore, clientApi: { storiesOf, addParameters }, - } = getContext(undefined); + } = getContext(); addParameters({ addon1: 'global string value', @@ -638,7 +667,7 @@ describe('preview.client_api', () => { const { clientApi: { storiesOf, getStorybook }, - } = getContext(undefined); + } = getContext(); expect(getStorybook()).toEqual([]); diff --git a/lib/client-api/src/client_api.ts b/lib/client-api/src/client_api.ts index af2c9db0844f..86e56f94ba38 100644 --- a/lib/client-api/src/client_api.ts +++ b/lib/client-api/src/client_api.ts @@ -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) { @@ -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); diff --git a/lib/client-api/src/types.ts b/lib/client-api/src/types.ts index d509750fb6c6..ae1289240dfe 100644 --- a/lib/client-api/src/types.ts +++ b/lib/client-api/src/types.ts @@ -29,6 +29,7 @@ export interface StoreData { export interface ClientApiParams { storyStore: StoryStore; decorateStory?: (storyFn: any, decorators: any) => any; + noStoryModuleAddMethodHotDispose?: boolean; } export type ClientApiReturnFn = (...args: any[]) => StoryApi;