Merge pull request #16487 from storybookjs/16486-fix-non-selected-hmr

Core: Always update initial args when loading a story
This commit is contained in:
Michael Shilman 2021-10-28 01:03:12 +08:00 committed by GitHub
commit 9814f81911
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 190 additions and 127 deletions

View File

@ -2233,6 +2233,78 @@ describe('PreviewWeb', () => {
});
});
describe('when another (not current) story changes', () => {
beforeEach(() => {
jest.useFakeTimers();
});
afterEach(() => {
jest.useRealTimers();
});
const newComponentOneExports = merge({}, componentOneExports, {
a: { args: { bar: 'edited' }, argTypes: { bar: { type: { name: 'string' } } } },
});
const newImportFn = jest.fn(async (path) => {
return path === './src/ComponentOne.stories.js'
? newComponentOneExports
: componentTwoExports;
});
it('retains the same delta to the args', async () => {
// Start at Story A
document.location.search = '?id=component-one--a';
const preview = await createAndRenderPreview();
// Change A's args
mockChannel.emit.mockClear();
emitter.emit(Events.UPDATE_STORY_ARGS, {
storyId: 'component-one--a',
updatedArgs: { foo: 'updated' },
});
await waitForRender();
// Change to story B
mockChannel.emit.mockClear();
emitter.emit(Events.SET_CURRENT_STORY, {
storyId: 'component-one--b',
viewMode: 'story',
});
await waitForSetCurrentStory();
await waitForRender();
expect(preview.storyStore.args.get('component-one--a')).toEqual({
foo: 'updated',
});
// Update story A's args via HMR
mockChannel.emit.mockClear();
projectAnnotations.renderToDOM.mockClear();
preview.onStoriesChanged({ importFn: newImportFn });
await waitForRender();
// Change back to Story A
mockChannel.emit.mockClear();
emitter.emit(Events.SET_CURRENT_STORY, {
storyId: 'component-one--a',
viewMode: 'story',
});
await waitForSetCurrentStory();
await waitForRender();
expect(preview.storyStore.args.get('component-one--a')).toEqual({
foo: 'updated',
bar: 'edited',
});
expect(projectAnnotations.renderToDOM).toHaveBeenCalledWith(
expect.objectContaining({
forceRemount: true,
storyContext: expect.objectContaining({
id: 'component-one--a',
args: { foo: 'updated', bar: 'edited' },
}),
}),
undefined // this is coming from view.prepareForStory, not super important
);
});
});
describe('if the story no longer exists', () => {
const { a, ...componentOneExportsWithoutA } = componentOneExports;
const newImportFn = jest.fn(async (path) => {

View File

@ -344,8 +344,6 @@ export class PreviewWeb<TFramework extends AnyFramework> {
if (persistedArgs) {
this.storyStore.args.updateFromPersisted(story, persistedArgs);
} else if (implementationChanged) {
this.storyStore.args.resetOnImplementationChange(story, this.previousStory);
}
// Don't re-render the story if nothing has changed to justify it

View File

@ -9,14 +9,7 @@ describe('ArgsStore', () => {
describe('setInitial / get', () => {
it('returns in a straightforward way', () => {
const store = new ArgsStore();
store.setInitial('id', { foo: 'bar' });
expect(store.get('id')).toEqual({ foo: 'bar' });
});
it('does not allow re-setting', () => {
const store = new ArgsStore();
store.setInitial('id', { foo: 'bar' });
store.setInitial('id', { foo: 'baz' });
store.setInitial({ id: 'id', initialArgs: { foo: 'bar' } } as any);
expect(store.get('id')).toEqual({ foo: 'bar' });
});
@ -24,13 +17,108 @@ describe('ArgsStore', () => {
const store = new ArgsStore();
expect(() => store.get('id')).toThrow(/No args known/);
});
describe('on second call for same story', () => {
describe('if initialArgs are unchanged', () => {
it('does nothing if the args are untouched', () => {
const store = new ArgsStore();
const previousStory = {
id: 'id',
initialArgs: { a: '1', b: '1' },
argTypes: { a: stringType, b: stringType },
} as any;
store.setInitial(previousStory);
const story = {
id: 'id',
initialArgs: { a: '1', b: '1' },
argTypes: { a: stringType, b: stringType },
} as any;
store.setInitial(story);
expect(store.get(story.id)).toEqual({ a: '1', b: '1' });
});
it('retains any arg changes', () => {
const store = new ArgsStore();
const previousStory = {
id: 'id',
initialArgs: { a: '1', b: false, c: 'unchanged' },
argTypes: { a: stringType, b: booleanType, c: stringType },
} as any;
store.setInitial(previousStory);
// NOTE: I'm not sure technically you should be allowed to set d here, but
// let's make sure we behave sensibly if you do
store.update('id', { a: 'update', b: true, d: 'update' });
const story = {
id: 'id',
initialArgs: { a: '1', b: false, c: 'unchanged' },
argTypes: { a: stringType, b: booleanType, c: stringType },
} as any;
store.setInitial(story);
// In any case c is not retained.
expect(store.get(story.id)).toEqual({ a: 'update', b: true, c: 'unchanged' });
});
});
describe('when initialArgs change', () => {
it('replaces old args with new if the args are untouched', () => {
const store = new ArgsStore();
const previousStory = {
id: 'id',
initialArgs: { a: '1', b: '1' },
argTypes: { a: stringType, b: stringType },
} as any;
store.setInitial(previousStory);
const story = {
id: 'id',
initialArgs: { a: '1', c: '1' },
argTypes: { a: stringType, c: stringType },
} as any;
store.setInitial(story);
expect(store.get(story.id)).toEqual({ a: '1', c: '1' });
});
it('applies the same delta if the args are changed', () => {
const store = new ArgsStore();
const previousStory = {
id: 'id',
initialArgs: { a: '1', b: '1' },
argTypes: { a: stringType, b: stringType },
} as any;
store.setInitial(previousStory);
// NOTE: I'm not sure technically you should be allowed to set c here
store.update('id', { a: 'update', c: 'update' });
const story = {
id: 'id',
initialArgs: { a: '2', d: '2' },
argTypes: { a: stringType, d: stringType },
} as any;
store.setInitial(story);
// In any case c is not retained.
expect(store.get(story.id)).toEqual({ a: 'update', d: '2' });
});
});
});
});
describe('update', () => {
it('overrides on a per-key basis', () => {
const store = new ArgsStore();
store.setInitial('id', {});
store.setInitial({ id: 'id', initialArgs: {} } as any);
store.update('id', { foo: 'bar' });
expect(store.get('id')).toEqual({ foo: 'bar' });
@ -42,7 +130,7 @@ describe('ArgsStore', () => {
it('does not merge objects', () => {
const store = new ArgsStore();
store.setInitial('id', {});
store.setInitial({ id: 'id', initialArgs: {} } as any);
store.update('id', { obj: { foo: 'bar' } });
expect(store.get('id')).toEqual({ obj: { foo: 'bar' } });
@ -54,7 +142,7 @@ describe('ArgsStore', () => {
it('does not set keys to undefined, it simply unsets them', () => {
const store = new ArgsStore();
store.setInitial('id', { foo: 'bar' });
store.setInitial({ id: 'id', initialArgs: { foo: 'bar' } } as any);
store.update('id', { foo: undefined });
expect('foo' in store.get('id')).toBe(false);
@ -65,7 +153,7 @@ describe('ArgsStore', () => {
it('ensures the types of args are correct', () => {
const store = new ArgsStore();
store.setInitial('id', {});
store.setInitial({ id: 'id', initialArgs: {} } as any);
const story = {
id: 'id',
@ -81,10 +169,7 @@ describe('ArgsStore', () => {
it('merges objects and sparse arrays', () => {
const store = new ArgsStore();
store.setInitial('id', {
a: { foo: 'bar' },
b: ['1', '2', '3'],
});
store.setInitial({ id: 'id', initialArgs: { a: { foo: 'bar' }, b: ['1', '2', '3'] } } as any);
const story = {
id: 'id',
@ -110,7 +195,7 @@ describe('ArgsStore', () => {
it('checks args are allowed options', () => {
const store = new ArgsStore();
store.setInitial('id', {});
store.setInitial({ id: 'id', initialArgs: {} } as any);
const story = {
id: 'id',
@ -123,99 +208,4 @@ describe('ArgsStore', () => {
expect(store.get('id')).toEqual({ a: 'a' });
});
});
describe('resetOnImplementationChange', () => {
describe('if initialArgs are unchanged', () => {
it('does nothing if the args are untouched', () => {
const store = new ArgsStore();
const previousStory = {
id: 'id',
initialArgs: { a: '1', b: '1' },
argTypes: { a: stringType, b: stringType },
} as any;
store.setInitial('id', previousStory.initialArgs);
const story = {
id: 'id',
initialArgs: { a: '1', b: '1' },
argTypes: { a: stringType, b: stringType },
} as any;
store.resetOnImplementationChange(story, previousStory);
expect(store.get(story.id)).toEqual({ a: '1', b: '1' });
});
it('retains any arg changes', () => {
const store = new ArgsStore();
const previousStory = {
id: 'id',
initialArgs: { a: '1', b: false, c: 'unchanged' },
argTypes: { a: stringType, b: booleanType, c: stringType },
} as any;
store.setInitial('id', previousStory.initialArgs);
// NOTE: I'm not sure technically you should be allowed to set d here, but
// let's make sure we behave sensibly if you do
store.update('id', { a: 'update', b: true, d: 'update' });
const story = {
id: 'id',
initialArgs: { a: '1', b: false, c: 'unchanged' },
argTypes: { a: stringType, b: booleanType, c: stringType },
} as any;
store.resetOnImplementationChange(story, previousStory);
// In any case c is not retained.
expect(store.get(story.id)).toEqual({ a: 'update', b: true, c: 'unchanged' });
});
});
describe('when initialArgs change', () => {
it('replaces old args with new if the args are untouched', () => {
const store = new ArgsStore();
const previousStory = {
id: 'id',
initialArgs: { a: '1', b: '1' },
argTypes: { a: stringType, b: stringType },
} as any;
store.setInitial('id', previousStory.initialArgs);
const story = {
id: 'id',
initialArgs: { a: '1', c: '1' },
argTypes: { a: stringType, c: stringType },
} as any;
store.resetOnImplementationChange(story, previousStory);
expect(store.get(story.id)).toEqual({ a: '1', c: '1' });
});
it('applies the same delta if the args are changed', () => {
const store = new ArgsStore();
const previousStory = {
id: 'id',
initialArgs: { a: '1', b: '1' },
argTypes: { a: stringType, b: stringType },
} as any;
store.setInitial('id', previousStory.initialArgs);
// NOTE: I'm not sure technically you should be allowed to set c here
store.update('id', { a: 'update', c: 'update' });
const story = {
id: 'id',
initialArgs: { a: '2', d: '2' },
argTypes: { a: stringType, d: stringType },
} as any;
store.resetOnImplementationChange(story, previousStory);
// In any case c is not retained.
expect(store.get(story.id)).toEqual({ a: 'update', d: '2' });
});
});
});
});

View File

@ -10,6 +10,8 @@ function deleteUndefined(obj: Record<string, any>) {
}
export class ArgsStore {
initialArgsByStoryId: Record<StoryId, Args> = {};
argsByStoryId: Record<StoryId, Args> = {};
get(storyId: StoryId) {
@ -20,9 +22,19 @@ export class ArgsStore {
return this.argsByStoryId[storyId];
}
setInitial(storyId: StoryId, args: Args) {
if (!this.argsByStoryId[storyId]) {
this.argsByStoryId[storyId] = args;
setInitial(story: Story<any>) {
if (!this.initialArgsByStoryId[story.id]) {
this.initialArgsByStoryId[story.id] = story.initialArgs;
this.argsByStoryId[story.id] = story.initialArgs;
} else if (this.initialArgsByStoryId[story.id] !== story.initialArgs) {
// When we get a new version of a story (with new initialArgs), we re-apply the same diff
// that we had previously applied to the old version of the story
const delta = deepDiff(this.initialArgsByStoryId[story.id], this.argsByStoryId[story.id]);
this.initialArgsByStoryId[story.id] = story.initialArgs;
this.argsByStoryId[story.id] = story.initialArgs;
if (delta !== DEEPLY_EQUAL) {
this.updateFromDelta(story, delta);
}
}
}
@ -44,15 +56,6 @@ export class ArgsStore {
return this.updateFromDelta(story, mappedPersisted);
}
resetOnImplementationChange(story: Story<any>, previousStory: Story<any>) {
const delta = deepDiff(previousStory.initialArgs, this.get(story.id));
this.argsByStoryId[story.id] = story.initialArgs;
if (delta !== DEEPLY_EQUAL) {
this.updateFromDelta(story, delta);
}
}
update(storyId: StoryId, argsUpdate: Partial<Args>) {
if (!(storyId in this.argsByStoryId)) {
throw new Error(`No args known for ${storyId} -- has it been rendered yet?`);

View File

@ -263,7 +263,7 @@ export class StoryStore<TFramework extends AnyFramework> {
componentAnnotations,
this.projectAnnotations
);
this.args.setInitial(story.id, story.initialArgs);
this.args.setInitial(story);
this.hooks[story.id] = this.hooks[story.id] || new HooksContext();
return story;
}