From 3b959d3d237a9bd3fe684a873ce6a54fd7710074 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Fri, 19 Apr 2024 18:22:16 +0200 Subject: [PATCH 1/4] add logs to try and debug flakiness --- .../core-server/src/presets/common-manager.ts | 1 + code/lib/manager-api/src/index.tsx | 22 +++++++++++++--- code/lib/manager-api/src/lib/stories.ts | 4 +++ code/lib/manager-api/src/modules/stories.ts | 26 ++++++++++++++++++- code/ui/manager/src/container/Sidebar.tsx | 1 + 5 files changed, 49 insertions(+), 5 deletions(-) diff --git a/code/lib/core-server/src/presets/common-manager.ts b/code/lib/core-server/src/presets/common-manager.ts index 0564f8e00b92..138f6064fcce 100644 --- a/code/lib/core-server/src/presets/common-manager.ts +++ b/code/lib/core-server/src/presets/common-manager.ts @@ -17,6 +17,7 @@ addons.register(STATIC_FILTER, (api) => { {} as Record ); + console.log('Registering STATIC_FILTER'); api.experimental_setFilter(STATIC_FILTER, (item) => { const tags = item.tags || []; return tags.filter((tag) => excludeTags[tag]).length === 0; diff --git a/code/lib/manager-api/src/index.tsx b/code/lib/manager-api/src/index.tsx index fbb8b74a6420..baa1874f672d 100644 --- a/code/lib/manager-api/src/index.tsx +++ b/code/lib/manager-api/src/index.tsx @@ -167,12 +167,19 @@ class ManagerProvider extends Component { navigate, } = props; + console.log('creating store', this.state); const store = new Store({ getState: () => this.state, - setState: (stateChange: Partial, callback) => { - this.setState(stateChange, () => callback(this.state)); - - return this.state; + setState: async (stateChange: Partial, callback) => { + // attempting to turn this into async to fix timing issues + return new Promise((resolve) => { + console.log('calling setState with stateChange', stateChange); + this.setState(stateChange, () => { + console.log('calling the callback of setState with ', this.state); + callback(this.state); + resolve(this.state); + }); + }); }, }); @@ -181,12 +188,14 @@ class ManagerProvider extends Component { this.state = store.getInitialState(getInitialState({ ...routeData, ...optionsData })); + console.log('initializing with state:', this.state); const apiData = { navigate, store, provider: props.provider, }; + console.log('initializing modules'); this.modules = [ provider, channel, @@ -208,6 +217,10 @@ class ManagerProvider extends Component { // Create our initial state by combining the initial state of all modules, then overlaying any saved state const state = getInitialState(this.state, ...this.modules.map((m) => m.state!)); + console.log('overriding with with state:', { + before: getInitialState(this.state), + after: state, + }); // Get our API by combining the APIs exported by each module const api: API = Object.assign(this.api, { navigate }, ...this.modules.map((m) => m.api)); @@ -217,6 +230,7 @@ class ManagerProvider extends Component { static getDerivedStateFromProps(props: ManagerProviderProps, state: State): State { if (state.path !== props.path) { + console.log('derived state is:', state); return { ...state, location: props.location, diff --git a/code/lib/manager-api/src/lib/stories.ts b/code/lib/manager-api/src/lib/stories.ts index 9d6b1817e677..ab52462f3827 100644 --- a/code/lib/manager-api/src/lib/stories.ts +++ b/code/lib/manager-api/src/lib/stories.ts @@ -162,6 +162,10 @@ export const transformStoryIndexToStoriesHash = ( const entryValues = Object.values(index.entries).filter((entry: any) => { let result = true; + console.log( + 'Time to filter out the index - ' + + (Object.keys(filters).length > 0 ? 'and there is a filter' : 'but there is no filter') + ); Object.values(filters).forEach((filter: any) => { if (result === false) { return; diff --git a/code/lib/manager-api/src/modules/stories.ts b/code/lib/manager-api/src/modules/stories.ts index 78c3307f179f..c8861f64f720 100644 --- a/code/lib/manager-api/src/modules/stories.ts +++ b/code/lib/manager-api/src/modules/stories.ts @@ -533,6 +533,7 @@ export const init: ModuleFn = ({ return; } + console.log('calling api.setIndex from api.fetchIndex'); await api.setIndex(storyIndex); } catch (err: any) { await store.setState({ indexError: err }); @@ -543,6 +544,12 @@ export const init: ModuleFn = ({ // so we can cast one to the other easily enough setIndex: async (input) => { const { index: oldHash, status, filters } = store.getState(); + console.log( + `api.setIndex calling transformStoryIndexToStoriesHash with ${ + Object.keys(filters).length + } filters` + ); + const newHash = transformStoryIndexToStoriesHash(input, { provider, docsOptions, @@ -639,6 +646,7 @@ export const init: ModuleFn = ({ if (index) { // We need to re-prepare the index + console.log('calling api.setIndex from experimental_updateStatus'); await api.setIndex(index); const refs = await fullAPI.getRefs(); @@ -649,9 +657,15 @@ export const init: ModuleFn = ({ }, experimental_setFilter: async (id, filterFunction) => { const { internal_index: index } = store.getState(); + console.log('experimental_setFilter inner function', { + index, + filters: store.getState().filters, + filterFunction, + }); await store.setState({ filters: { ...store.getState().filters, [id]: filterFunction } }); if (index) { + console.log('calling api.setIndex from experimental_setFilter'); await api.setIndex(index); const refs = await fullAPI.getRefs(); @@ -781,6 +795,7 @@ export const init: ModuleFn = ({ const { ref } = getEventMetadata(this, fullAPI)!; if (!ref) { + console.log('calling api.setIndex from SET_INDEX listener when there is no ref'); api.setIndex(index); const options = api.getCurrentParameter('options'); fullAPI.setOptions(removeRemovedOptions(options!)); @@ -855,7 +870,9 @@ export const init: ModuleFn = ({ provider.channel?.on(SET_CONFIG, () => { const config = provider.getConfig(); + console.log('getting config'); if (config?.sidebar?.filters) { + console.log("there are filters, let's set them"); store.setState({ filters: { ...store.getState().filters, @@ -867,6 +884,7 @@ export const init: ModuleFn = ({ const config = provider.getConfig(); + console.log('setting initial state'); return { api, state: { @@ -878,8 +896,14 @@ export const init: ModuleFn = ({ filters: config?.sidebar?.filters || {}, }, init: async () => { - provider.channel?.on(STORY_INDEX_INVALIDATED, () => api.fetchIndex()); + provider.channel?.on(STORY_INDEX_INVALIDATED, () => { + console.log('calling api.fetchIndex from STORY_INDEX_INVALIDATED event'); + api.fetchIndex(); + }); + + console.log('calling api.fetchIndex from stories module init - started'); await api.fetchIndex(); + console.log('calling api.fetchIndex from stories module init - completed'); }, }; }; diff --git a/code/ui/manager/src/container/Sidebar.tsx b/code/ui/manager/src/container/Sidebar.tsx index 21d9cf09ef6c..6bbe2e1f1a83 100755 --- a/code/ui/manager/src/container/Sidebar.tsx +++ b/code/ui/manager/src/container/Sidebar.tsx @@ -49,6 +49,7 @@ const Sidebar = React.memo(function Sideber({ onMenuClick }: SidebarProps) { // eslint-disable-next-line react-hooks/exhaustive-deps const top = useMemo(() => Object.values(topItems), [Object.keys(topItems).join('')]); + console.log('Sidebar context mapper - ', { indexCount: index ? Object.keys(index).length : 0 }); return { title: name, url, From a092fedc1ed4e5541fbf05616c66558053167523 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 25 Apr 2024 11:30:27 +0200 Subject: [PATCH 2/4] get index AFTER setting filters is done This ensures that when checking if there is an index to reset, it's not using stale data from before the index was even fully fetched. --- code/lib/manager-api/src/modules/stories.ts | 47 ++++++++++++++------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/code/lib/manager-api/src/modules/stories.ts b/code/lib/manager-api/src/modules/stories.ts index c8861f64f720..c1672e7873c5 100644 --- a/code/lib/manager-api/src/modules/stories.ts +++ b/code/lib/manager-api/src/modules/stories.ts @@ -522,10 +522,13 @@ export const init: ModuleFn = ({ }, fetchIndex: async () => { try { + const now = Date.now(); + console.log('LOG: fetchIndex before fetch'); const result = await fetch(STORY_INDEX_PATH); if (result.status !== 200) throw new Error(await result.text()); const storyIndex = (await result.json()) as StoryIndex; + console.log('LOG: fetchIndex after fetch', { storyIndex, bench: Date.now() - now }); // We can only do this if the stories.json is a proper storyIndex if (storyIndex.v < 3) { @@ -543,9 +546,26 @@ export const init: ModuleFn = ({ // The story index we receive on fetchStoryIndex is not, but all the prepared fields are optional // so we can cast one to the other easily enough setIndex: async (input) => { - const { index: oldHash, status, filters } = store.getState(); + const state = store.getState(); + + /** + * The initial setIndex (as called by fetchIndex) is in a race with this.setSate in the ManagerProvider + * Especially in Webkit, fetchIndex is done long before filters have been set in the state + * But we always expect the internal 'static-filter' to be set + */ + // for (let i = 0; i < 10; i++) { + // if (state.filters['static-filter']) { + // break; + // } + // state = await new Promise((resolve) => { + // setTimeout(() => resolve(store.getState()), 0); + // }); + // } + + const { index: oldHash, status, filters } = state; + console.log( - `api.setIndex calling transformStoryIndexToStoriesHash with ${ + `!!!!!!! api.setIndex calling transformStoryIndexToStoriesHash with ${ Object.keys(filters).length } filters` ); @@ -656,23 +676,20 @@ export const init: ModuleFn = ({ } }, experimental_setFilter: async (id, filterFunction) => { - const { internal_index: index } = store.getState(); - console.log('experimental_setFilter inner function', { - index, - filters: store.getState().filters, - filterFunction, - }); await store.setState({ filters: { ...store.getState().filters, [id]: filterFunction } }); - if (index) { - console.log('calling api.setIndex from experimental_setFilter'); - await api.setIndex(index); + const { internal_index: index } = store.getState(); - const refs = await fullAPI.getRefs(); - Object.entries(refs).forEach(([refId, { internal_index, ...ref }]) => { - fullAPI.setRef(refId, { ...ref, storyIndex: internal_index }, true); - }); + if (!index) { + return; } + // apply new filters by setting the index again + await api.setIndex(index); + + const refs = await fullAPI.getRefs(); + Object.entries(refs).forEach(([refId, { internal_index, ...ref }]) => { + fullAPI.setRef(refId, { ...ref, storyIndex: internal_index }, true); + }); }, }; From 3cbe01d3ccc5927f47e640666d1bfe1ecce76fda Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 25 Apr 2024 11:35:00 +0200 Subject: [PATCH 3/4] cleanup --- .../core-server/src/presets/common-manager.ts | 1 - code/lib/manager-api/src/index.tsx | 22 +++--------- code/lib/manager-api/src/lib/stories.ts | 4 --- code/lib/manager-api/src/modules/stories.ts | 36 +------------------ code/ui/manager/src/container/Sidebar.tsx | 1 - 5 files changed, 5 insertions(+), 59 deletions(-) diff --git a/code/lib/core-server/src/presets/common-manager.ts b/code/lib/core-server/src/presets/common-manager.ts index 138f6064fcce..0564f8e00b92 100644 --- a/code/lib/core-server/src/presets/common-manager.ts +++ b/code/lib/core-server/src/presets/common-manager.ts @@ -17,7 +17,6 @@ addons.register(STATIC_FILTER, (api) => { {} as Record ); - console.log('Registering STATIC_FILTER'); api.experimental_setFilter(STATIC_FILTER, (item) => { const tags = item.tags || []; return tags.filter((tag) => excludeTags[tag]).length === 0; diff --git a/code/lib/manager-api/src/index.tsx b/code/lib/manager-api/src/index.tsx index baa1874f672d..fbb8b74a6420 100644 --- a/code/lib/manager-api/src/index.tsx +++ b/code/lib/manager-api/src/index.tsx @@ -167,19 +167,12 @@ class ManagerProvider extends Component { navigate, } = props; - console.log('creating store', this.state); const store = new Store({ getState: () => this.state, - setState: async (stateChange: Partial, callback) => { - // attempting to turn this into async to fix timing issues - return new Promise((resolve) => { - console.log('calling setState with stateChange', stateChange); - this.setState(stateChange, () => { - console.log('calling the callback of setState with ', this.state); - callback(this.state); - resolve(this.state); - }); - }); + setState: (stateChange: Partial, callback) => { + this.setState(stateChange, () => callback(this.state)); + + return this.state; }, }); @@ -188,14 +181,12 @@ class ManagerProvider extends Component { this.state = store.getInitialState(getInitialState({ ...routeData, ...optionsData })); - console.log('initializing with state:', this.state); const apiData = { navigate, store, provider: props.provider, }; - console.log('initializing modules'); this.modules = [ provider, channel, @@ -217,10 +208,6 @@ class ManagerProvider extends Component { // Create our initial state by combining the initial state of all modules, then overlaying any saved state const state = getInitialState(this.state, ...this.modules.map((m) => m.state!)); - console.log('overriding with with state:', { - before: getInitialState(this.state), - after: state, - }); // Get our API by combining the APIs exported by each module const api: API = Object.assign(this.api, { navigate }, ...this.modules.map((m) => m.api)); @@ -230,7 +217,6 @@ class ManagerProvider extends Component { static getDerivedStateFromProps(props: ManagerProviderProps, state: State): State { if (state.path !== props.path) { - console.log('derived state is:', state); return { ...state, location: props.location, diff --git a/code/lib/manager-api/src/lib/stories.ts b/code/lib/manager-api/src/lib/stories.ts index ab52462f3827..9d6b1817e677 100644 --- a/code/lib/manager-api/src/lib/stories.ts +++ b/code/lib/manager-api/src/lib/stories.ts @@ -162,10 +162,6 @@ export const transformStoryIndexToStoriesHash = ( const entryValues = Object.values(index.entries).filter((entry: any) => { let result = true; - console.log( - 'Time to filter out the index - ' + - (Object.keys(filters).length > 0 ? 'and there is a filter' : 'but there is no filter') - ); Object.values(filters).forEach((filter: any) => { if (result === false) { return; diff --git a/code/lib/manager-api/src/modules/stories.ts b/code/lib/manager-api/src/modules/stories.ts index c1672e7873c5..7903f283228a 100644 --- a/code/lib/manager-api/src/modules/stories.ts +++ b/code/lib/manager-api/src/modules/stories.ts @@ -522,13 +522,10 @@ export const init: ModuleFn = ({ }, fetchIndex: async () => { try { - const now = Date.now(); - console.log('LOG: fetchIndex before fetch'); const result = await fetch(STORY_INDEX_PATH); if (result.status !== 200) throw new Error(await result.text()); const storyIndex = (await result.json()) as StoryIndex; - console.log('LOG: fetchIndex after fetch', { storyIndex, bench: Date.now() - now }); // We can only do this if the stories.json is a proper storyIndex if (storyIndex.v < 3) { @@ -536,7 +533,6 @@ export const init: ModuleFn = ({ return; } - console.log('calling api.setIndex from api.fetchIndex'); await api.setIndex(storyIndex); } catch (err: any) { await store.setState({ indexError: err }); @@ -546,29 +542,7 @@ export const init: ModuleFn = ({ // The story index we receive on fetchStoryIndex is not, but all the prepared fields are optional // so we can cast one to the other easily enough setIndex: async (input) => { - const state = store.getState(); - - /** - * The initial setIndex (as called by fetchIndex) is in a race with this.setSate in the ManagerProvider - * Especially in Webkit, fetchIndex is done long before filters have been set in the state - * But we always expect the internal 'static-filter' to be set - */ - // for (let i = 0; i < 10; i++) { - // if (state.filters['static-filter']) { - // break; - // } - // state = await new Promise((resolve) => { - // setTimeout(() => resolve(store.getState()), 0); - // }); - // } - - const { index: oldHash, status, filters } = state; - - console.log( - `!!!!!!! api.setIndex calling transformStoryIndexToStoriesHash with ${ - Object.keys(filters).length - } filters` - ); + const { index: oldHash, status, filters } = store.getState(); const newHash = transformStoryIndexToStoriesHash(input, { provider, @@ -666,7 +640,6 @@ export const init: ModuleFn = ({ if (index) { // We need to re-prepare the index - console.log('calling api.setIndex from experimental_updateStatus'); await api.setIndex(index); const refs = await fullAPI.getRefs(); @@ -812,7 +785,6 @@ export const init: ModuleFn = ({ const { ref } = getEventMetadata(this, fullAPI)!; if (!ref) { - console.log('calling api.setIndex from SET_INDEX listener when there is no ref'); api.setIndex(index); const options = api.getCurrentParameter('options'); fullAPI.setOptions(removeRemovedOptions(options!)); @@ -887,9 +859,7 @@ export const init: ModuleFn = ({ provider.channel?.on(SET_CONFIG, () => { const config = provider.getConfig(); - console.log('getting config'); if (config?.sidebar?.filters) { - console.log("there are filters, let's set them"); store.setState({ filters: { ...store.getState().filters, @@ -901,7 +871,6 @@ export const init: ModuleFn = ({ const config = provider.getConfig(); - console.log('setting initial state'); return { api, state: { @@ -914,13 +883,10 @@ export const init: ModuleFn = ({ }, init: async () => { provider.channel?.on(STORY_INDEX_INVALIDATED, () => { - console.log('calling api.fetchIndex from STORY_INDEX_INVALIDATED event'); api.fetchIndex(); }); - console.log('calling api.fetchIndex from stories module init - started'); await api.fetchIndex(); - console.log('calling api.fetchIndex from stories module init - completed'); }, }; }; diff --git a/code/ui/manager/src/container/Sidebar.tsx b/code/ui/manager/src/container/Sidebar.tsx index 6bbe2e1f1a83..21d9cf09ef6c 100755 --- a/code/ui/manager/src/container/Sidebar.tsx +++ b/code/ui/manager/src/container/Sidebar.tsx @@ -49,7 +49,6 @@ const Sidebar = React.memo(function Sideber({ onMenuClick }: SidebarProps) { // eslint-disable-next-line react-hooks/exhaustive-deps const top = useMemo(() => Object.values(topItems), [Object.keys(topItems).join('')]); - console.log('Sidebar context mapper - ', { indexCount: index ? Object.keys(index).length : 0 }); return { title: name, url, From 65c84db98a5a584a44e61786a7b69b8ed8a0b165 Mon Sep 17 00:00:00 2001 From: Jeppe Reinhold Date: Thu, 25 Apr 2024 11:36:17 +0200 Subject: [PATCH 4/4] cleanup --- code/lib/manager-api/src/modules/stories.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/code/lib/manager-api/src/modules/stories.ts b/code/lib/manager-api/src/modules/stories.ts index 7903f283228a..19ea80ce41f4 100644 --- a/code/lib/manager-api/src/modules/stories.ts +++ b/code/lib/manager-api/src/modules/stories.ts @@ -543,7 +543,6 @@ export const init: ModuleFn = ({ // so we can cast one to the other easily enough setIndex: async (input) => { const { index: oldHash, status, filters } = store.getState(); - const newHash = transformStoryIndexToStoriesHash(input, { provider, docsOptions, @@ -882,9 +881,7 @@ export const init: ModuleFn = ({ filters: config?.sidebar?.filters || {}, }, init: async () => { - provider.channel?.on(STORY_INDEX_INVALIDATED, () => { - api.fetchIndex(); - }); + provider.channel?.on(STORY_INDEX_INVALIDATED, () => api.fetchIndex()); await api.fetchIndex(); },