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.

Sign up for GitHub

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

Core: Add skip dispose option to ClientApi #9868

Merged
merged 7 commits into from
Feb 19, 2020
Merged

Core: Add skip dispose option to ClientApi #9868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
23da639
added ability to skip dispose call in clientApi
Gongreg Feb 15, 2020
7753bfa
updated config name, added comment, fixed tests
Gongreg Feb 16, 2020
b2e8d0d
Update lib/client-api/src/client_api.test.ts
Gongreg Feb 16, 2020
9df7afc
Update client_api.ts
Gongreg Feb 16, 2020
83ec145
yarn.lock
Gongreg Feb 19, 2020
e921895
Merge branch 'client-api-hot-refresh' of github.com:storybooks/storyb…
Gongreg Feb 19, 2020
fe1d458
Update types.ts
Gongreg Feb 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 62 additions & 33 deletions lib/client-api/src/client_api.test.ts
View file
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,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 @@ -206,7 +215,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
Open in desktop
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
Loading