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.

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: Show "booting" progress until story is specified or errors #20425

Merged
merged 34 commits into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
2341c67
Show a "booting" progress message until the story is specified
tmeasday Dec 28, 2022
91a431a
Apply suggestions from code review
tmeasday Dec 29, 2022
4cdecb9
Don't show a progressbar if booting
tmeasday Dec 29, 2022
38fcce2
Simplify things further
tmeasday Dec 29, 2022
e91aa76
Simplify a little
tmeasday Dec 29, 2022
8756b5d
Simplify
tmeasday Dec 29, 2022
9d2a9b5
Add some comments to explain things
tmeasday Jan 2, 2023
8f60c3d
Remove `storiesConfigured` et al, and simply to just `ready`.
tmeasday Jan 3, 2023
7ec1104
Show docs immediately on first load
tmeasday Jan 3, 2023
5575b4d
Don't set ready until we are actually ready!
tmeasday Jan 8, 2023
b839502
Change refs + stories to use a common interface
tmeasday Jan 11, 2023
5f105af
Drop some logs
tmeasday Jan 11, 2023
1714df8
Update tests
tmeasday Jan 12, 2023
4052c46
Missed this
tmeasday Jan 12, 2023
242a87b
Update snapshots
tmeasday Jan 12, 2023
93e9558
Update various tests/stories
tmeasday Jan 12, 2023
6ebf577
One more
tmeasday Jan 12, 2023
68f1975
Merge remote-tracking branch 'origin/next' into tom/sb-1123-sb20250-v…
tmeasday Jan 12, 2023
7358d55
Merge branch 'next' into tom/sb-1123-sb20250-vite-does-not-show-a-spi…
ndelangen Jan 12, 2023
48828c7
make changes to typings files to ensure this the check command succeeds
ndelangen Jan 12, 2023
4ed7408
Update stories->index and add back-compatibility
tmeasday Jan 13, 2023
d6fa356
Merge branch 'tom/sb-1123-sb20250-vite-does-not-show-a-spinner-2' of …
tmeasday Jan 13, 2023
24f35c5
Various updates
tmeasday Jan 13, 2023
0ae65e8
More fixes
tmeasday Jan 13, 2023
668565a
Fix up app stories to be initialized
tmeasday Jan 13, 2023
b25ec31
Bunch of updates I missed
tmeasday Jan 13, 2023
f5dba59
Missed one more
tmeasday Jan 13, 2023
996fdde
Even more fixes
tmeasday Jan 13, 2023
39f5da8
Another issue
tmeasday Jan 13, 2023
c65b8ab
Catch missing stories
tmeasday Jan 13, 2023
02cee42
Add debugging to built-in sb
tmeasday Jan 13, 2023
dead68d
Merge pull request #20598 from storybookjs/norbert/make-typescript-ch…
ndelangen Jan 13, 2023
d972473
Merge branch 'next' into tom/sb-1123-sb20250-vite-does-not-show-a-spi…
ndelangen Jan 18, 2023
f3bfca4
when testing for regressions, this didn't show up at all. renaming it…
ndelangen Jan 18, 2023
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
45 changes: 28 additions & 17 deletions code/lib/manager-api/src/modules/stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
STORY_SPECIFIED,
STORY_INDEX_INVALIDATED,
CONFIG_ERROR,
CURRENT_STORY_WAS_SET,
} from '@storybook/core-events';
import { logger } from '@storybook/client-logger';

Expand Down Expand Up @@ -54,8 +55,7 @@ export interface SubState {
storiesHash: API_StoriesHash;
storyId: StoryId;
viewMode: ViewMode;
storiesConfigured: boolean;
storiesFailed?: Error;
tmeasday marked this conversation as resolved.
Show resolved Hide resolved
ready: boolean;
}

export interface SubAPI {
Expand Down Expand Up @@ -325,10 +325,7 @@ export const init: ModuleFn<SubAPI, SubState, true> = ({

await fullAPI.setIndex(storyIndex);
} catch (err) {
store.setState({
storiesConfigured: true,
storiesFailed: err,
});
store.setState({ ready: true });
}
},
// The story index we receive on SET_INDEX is "prepared" in that it has parameters
Expand All @@ -345,8 +342,7 @@ export const init: ModuleFn<SubAPI, SubState, true> = ({

await store.setState({
storiesHash: addPreparedStories(newHash, oldHash),
storiesConfigured: true,
storiesFailed: null,
ready: true,
});
},
updateStory: async (
Expand Down Expand Up @@ -387,9 +383,9 @@ export const init: ModuleFn<SubAPI, SubState, true> = ({
}) {
const { sourceType } = getEventMetadata(this, fullAPI);

if (fullAPI.isSettingsScreenActive()) return;

if (sourceType === 'local') {
if (fullAPI.isSettingsScreenActive()) return;

// Special case -- if we are already at the story being specified (i.e. the user started at a given story),
// we don't need to change URL. See https://github.com/storybookjs/storybook/issues/11677
const state = store.getState();
Expand All @@ -400,6 +396,20 @@ export const init: ModuleFn<SubAPI, SubState, true> = ({
}
);

// The CURRENT_STORY_WAS_SET event is the best event to use to tell if a ref is ready.
// Until the ref has a selection, it will not render anything (e.g. while waiting for
// the preview.js file or the index to load). Once it has a selection, it will render its own
// preparing spinner.
fullAPI.on(CURRENT_STORY_WAS_SET, function handler() {
const { ref } = getEventMetadata(this, fullAPI);

if (!ref) {
store.setState({ ready: true });
} else {
fullAPI.updateRef(ref.id, { ready: true });
}
});

fullAPI.on(STORY_CHANGED, function handler() {
const { sourceType } = getEventMetadata(this, fullAPI);

Expand All @@ -422,8 +432,6 @@ export const init: ModuleFn<SubAPI, SubState, true> = ({
fullAPI.setOptions(removeRemovedOptions(options));
store.setState({ hasCalledSetOptions: true });
}
} else {
fullAPI.updateRef(ref.id, { ready: true });
}

if (sourceType === 'local') {
Expand Down Expand Up @@ -500,10 +508,13 @@ export const init: ModuleFn<SubAPI, SubState, true> = ({
);

fullAPI.on(CONFIG_ERROR, function handleConfigError(err) {
store.setState({
storiesConfigured: true,
storiesFailed: err,
});
const { ref } = getEventMetadata(this, fullAPI);

if (!ref) {
store.setState({ ready: true });
} else {
fullAPI.updateRef(ref.id, { ready: true });
}
});

if (FEATURES?.storyStoreV7) {
Expand All @@ -518,7 +529,7 @@ export const init: ModuleFn<SubAPI, SubState, true> = ({
storiesHash: {},
storyId: initialStoryId,
viewMode: initialViewMode,
storiesConfigured: false,
ready: false,
hasCalledSetOptions: false,
},
init: initModule,
Expand Down
94 changes: 59 additions & 35 deletions code/lib/manager-api/src/tests/stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
STORY_INDEX_INVALIDATED,
CONFIG_ERROR,
SET_INDEX,
CURRENT_STORY_WAS_SET,
} from '@storybook/core-events';
import { EventEmitter } from 'events';
import { global } from '@storybook/global';
Expand Down Expand Up @@ -107,7 +108,7 @@ describe('stories API', () => {
} as ModuleArgs);

expect(state).toEqual({
storiesConfigured: false,
ready: false,
storiesHash: {},
storyId: 'id',
viewMode: 'story',
Expand Down Expand Up @@ -575,9 +576,8 @@ describe('stories API', () => {

await init();

const { storiesConfigured, storiesFailed } = store.getState();
expect(storiesConfigured).toBe(true);
expect(storiesFailed?.message).toMatch(/sorting error/);
const { ready } = store.getState();
expect(ready).toBe(true);
});

it('watches for the INVALIDATE event and refetches -- and resets the hash', async () => {
Expand Down Expand Up @@ -615,7 +615,6 @@ describe('stories API', () => {
});
});

// Can't currently run these tests as cannot set this on the events
describe('STORY_SPECIFIED event', () => {
it('navigates to the story', async () => {
const navigate = jest.fn();
Expand Down Expand Up @@ -665,6 +664,43 @@ describe('stories API', () => {
});
});

describe('CURRENT_STORY_WAS_SET event', () => {
it('sets the local ready state', async () => {
const navigate = jest.fn();
const fullAPI = Object.assign(new EventEmitter());
const store = createMockStore({});
const { init, api } = initStories({ store, navigate, provider, fullAPI } as any);

Object.assign(fullAPI, api);
await init();
fullAPI.emit(CURRENT_STORY_WAS_SET, { id: 'a--1' });

expect(store.getState().ready).toBe(true);
});

it('sets a ref to ready', async () => {
const navigate = jest.fn();
const fullAPI = Object.assign(new EventEmitter(), { updateRef: jest.fn() });
const store = createMockStore();
const { api, init } = initStories({ store, navigate, provider, fullAPI } as any);

Object.assign(fullAPI, api);

getEventMetadataMock.mockReturnValueOnce({
sourceType: 'external',
ref: { id: 'refId', stories: { 'a--1': { args: { a: 'b' } } } },
} as any);
await init();
fullAPI.emit(CURRENT_STORY_WAS_SET, { id: 'a--1' });

expect(fullAPI.updateRef.mock.calls.length).toBe(1);

expect(fullAPI.updateRef.mock.calls[0][1]).toEqual({
ready: true,
});
});
});

describe('args handling', () => {
const parameters = {};
const preparedEntries: API_PreparedStoryIndex['entries'] = {
Expand Down Expand Up @@ -1284,56 +1320,44 @@ describe('stories API', () => {

expect(fullAPI.setOptions).not.toHaveBeenCalled();
});
});

it('sets the ref to ready when it is an external story', async () => {
describe('CONFIG_ERROR', () => {
it('sets ready to true, local', async () => {
const navigate = jest.fn();
const store = createMockStore();
const fullAPI = Object.assign(new EventEmitter(), {
setStories: jest.fn(),
updateRef: jest.fn(),
});
const fullAPI = Object.assign(new EventEmitter(), {});

const { api, init } = initStories({ store, navigate, provider, fullAPI } as any);
Object.assign(fullAPI, api);

getEventMetadataMock.mockReturnValueOnce({
sourceType: 'external',
ref: { id: 'refId', stories: { 'a--1': { args: { a: 'b' } } } },
} as any);
await init();

fullAPI.emit(STORY_PREPARED, {
id: 'a--1',
});

expect(fullAPI.updateRef.mock.calls.length).toBe(2);

expect(fullAPI.updateRef.mock.calls[0][1]).toEqual({
stories: { 'a--1': { args: { a: 'b' }, prepared: true } },
});
fullAPI.emit(CONFIG_ERROR, { message: 'Failed to run configure' });

expect(fullAPI.updateRef.mock.calls[1][1]).toEqual({
ready: true,
});
const { ready } = store.getState();
expect(ready).toBe(true);
});
});

describe('CONFIG_ERROR', () => {
it('shows error to user', async () => {
it('sets ready to true, ref', async () => {
const navigate = jest.fn();
const fullAPI = Object.assign(new EventEmitter(), { updateRef: jest.fn() });
const store = createMockStore();
const fullAPI = Object.assign(new EventEmitter(), {});

const { api, init } = initStories({ store, navigate, provider, fullAPI } as any);

Object.assign(fullAPI, api);

getEventMetadataMock.mockReturnValueOnce({
sourceType: 'external',
ref: { id: 'refId', stories: { 'a--1': { args: { a: 'b' } } } },
} as any);
await init();

fullAPI.emit(CONFIG_ERROR, { message: 'Failed to run configure' });

const { storiesConfigured, storiesFailed } = store.getState();
expect(storiesConfigured).toBe(true);
expect(storiesFailed?.message).toMatch(/Failed to run configure/);
expect(fullAPI.updateRef.mock.calls.length).toBe(1);
expect(fullAPI.updateRef.mock.calls[0][1]).toEqual({
ready: true,
});
});
});

Expand Down
2 changes: 1 addition & 1 deletion code/ui/manager/src/components/layout/app.mockdata.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const realSidebarProps: SidebarProps = {
stories: mockDataset.withRoot as SidebarProps['stories'],
menu: [],
refs: {},
storiesConfigured: true,
ready: true,
};

const PlaceholderBlock = styled.div(({ color }) => ({
Expand Down
17 changes: 7 additions & 10 deletions code/ui/manager/src/components/preview/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ const canvasMapper = ({ state, api }: Combo) => ({
queryParams: state.customQueryParams,
getElements: api.getElements,
entry: api.getData(state.storyId, state.refId),
storiesConfigured: state.storiesConfigured,
storiesFailed: state.storiesFailed,
ready: state.ready,
refs: state.refs,
active: !!(state.viewMode && state.viewMode.match(/^(story|docs)$/)),
});
Expand All @@ -60,8 +59,7 @@ const createCanvas = (id: string, baseUrl = 'iframe.html', withLoader = true): A
viewMode,
queryParams,
getElements,
storiesConfigured,
storiesFailed,
ready,
active,
}) => {
const wrappers = useMemo(
Expand All @@ -70,7 +68,6 @@ const createCanvas = (id: string, baseUrl = 'iframe.html', withLoader = true): A
);

const [progress, setProgress] = useState(undefined);

useEffect(() => {
if (FEATURES?.storyStoreV7 && global.CONFIG_TYPE === 'DEVELOPMENT') {
try {
Expand All @@ -84,12 +81,12 @@ const createCanvas = (id: string, baseUrl = 'iframe.html', withLoader = true): A
}
}
}, []);

// A ref simply depends on its readiness
const refLoading = !!refs[refId] && !refs[refId].ready;
const rootLoading = !refId && !(progress?.value === 1 || progress === undefined);
const isLoading = entry
? refLoading || rootLoading
: (!storiesFailed && !storiesConfigured) || rootLoading;
// The root also might need to wait on webpack
const isBuilding = !(progress?.value === 1 || progress === undefined);
const rootLoading = !refId && (!ready || isBuilding);
const isLoading = entry ? refLoading || rootLoading : rootLoading;

return (
<ZoomConsumer>
Expand Down
31 changes: 5 additions & 26 deletions code/ui/manager/src/components/sidebar/Sidebar.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,45 +38,24 @@ const refs: Record<string, RefType> = {
};

export const Simple = () => (
<Sidebar
storiesConfigured
menu={menu}
stories={stories as any}
storyId={storyId}
refId={refId}
refs={{}}
/>
<Sidebar ready menu={menu} stories={stories as any} storyId={storyId} refId={refId} refs={{}} />
);

export const Loading = () => (
<Sidebar
storiesConfigured={false}
menu={menu}
stories={{}}
storyId={storyId}
refId={refId}
refs={{}}
/>
<Sidebar ready={false} menu={menu} stories={{}} storyId={storyId} refId={refId} refs={{}} />
);

export const Empty = () => (
<Sidebar storiesConfigured menu={menu} stories={{}} storyId={storyId} refId={refId} refs={{}} />
<Sidebar ready menu={menu} stories={{}} storyId={storyId} refId={refId} refs={{}} />
);

export const WithRefs = () => (
<Sidebar
storiesConfigured
menu={menu}
stories={stories as any}
storyId={storyId}
refId={refId}
refs={refs}
/>
<Sidebar ready menu={menu} stories={stories as any} storyId={storyId} refId={refId} refs={refs} />
);

export const LoadingWithRefs = () => (
<Sidebar
storiesConfigured={false}
ready={false}
menu={menu}
stories={stories as any}
storyId={storyId}
Expand Down
Loading