imodeljs / itwin-viewer

Monorepo that contains the iTwin Viewer npm package and its related packages
MIT License
15 stars 13 forks source link

"Redux Store has not been initialized" if UiFramework initialized with custom store #62

Closed MichaelBelousov closed 4 years ago

MichaelBelousov commented 4 years ago

Maybe I'm doing something wrong here, but I am initializing UiFramework with a custom store instance, and as a result the Viewer component throws this error:

BentleyError.ts:400 Uncaught Error: Redux Store has not been initialized
    at Function.get store [as store] (StateManager.ts:89)
    at IModelLoader.tsx:247
    at renderWithHooks (react-dom.development.js:14803)
    at updateFunctionComponent (react-dom.development.js:17034)
    at updateSimpleMemoComponent (react-dom.development.js:16972)
    at beginWork (react-dom.development.js:18687)
    at HTMLUnknownElement.callCallback (react-dom.development.js:188)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:237)
    at invokeGuardedCallback (react-dom.development.js:292)
    at beginWork$1 (react-dom.development.js:23203)
    at performUnitOfWork (react-dom.development.js:22157)
    at workLoopSync (react-dom.development.js:22130)
    at performSyncWorkOnRoot (react-dom.development.js:21756)
    at react-dom.development.js:11089
    at unstable_runWithPriority (scheduler.development.js:653)
    at runWithPriority$1 (react-dom.development.js:11039)
    at flushSyncCallbackQueueImpl (react-dom.development.js:11084)
    at flushSyncCallbackQueue (react-dom.development.js:11072)
    at scheduleUpdateOnFiber (react-dom.development.js:21199)
    at dispatchAction (react-dom.development.js:15660)
    at getModelConnection (IModelLoader.tsx:161)

My cursory understanding is that itwin-viewer-react erroneously uses StateManager's store reference when it should be using UiFramework's store reference which allows both. Otherwise I don't know much about StateManager's role.

I am using this workaround in the meanwhile:

await UiFramework.initialize(store, IModelApp.i18n);
// SEE: https://github.com/imodeljs/viewer-components-react/issues/74
(StateManager as any)._singletonStore = store;
aruniverse commented 4 years ago

Im surprised you're hitting this error, as the itwin-viewer should in fact be initializing UiFramework, look here.

One thing we should probably be doing is exposing the StateManager so consumers can add their own stores to it.

MichaelBelousov commented 4 years ago

this seems to be the culprit to me https://github.com/imodeljs/itwin-viewer/blob/f255deea50a678140a4a6ac28b04dcb615650614/packages/modules/itwin-viewer-react/src/components/iModel/IModelLoader.tsx#L247 If you can use the UiFramework.store instead, it would default to StateManager.store if no custom store is used. But that might be naive, I'm not sure what else is going on

kckst8 commented 4 years ago

@aruniverse hit the nail on the head here. The main idea behind the viewer is that consumers should not need to worry about initializing all of the statics in iModel.js. It assumes that consumers would not be calling initialize. That said, we still want to support parameters for the statics where necessary. It seems like maybe we should allow for users to pass in custom stores?

We can also change the above code if you think that will fix your issues in the short term. Have you verified that this resolves it?

MichaelBelousov commented 4 years ago

I think the best option is to be able to pass a custom store which is what I initially thought would be a prop of the viewer.

I do initialize everything myself which I sort of didn't realize I didn't need to do, I have custom rpcs and static packages that need to be initialized and I could do them in the post-startup callback prop, but I need to prune my initialization logic then.

aruniverse commented 4 years ago

I think Mike should initialize his customs rpcs and static in the post-startup callback in order to not initialize any of the core imjs statics. If theres specific parameters to the core statics he needs to override, we should expose that.

Rather than passing in a custom store to use in the Provider in IModelLoader, I think we should try to make use of Core's ReducerRegistry so we can dynamically add to the StateManager.

MichaelBelousov commented 4 years ago

Out of scope of this thread but while I'm changing it, is it not possible to pass a custom i18n instance to IModelApp.startup using itwin-viewer? It looks like it's always explicitly created for you. I'd like to be able to use translations before the Viewer component is rendered, any suggestions? I18next is static so it's possible but I'm not sure how that interacts with IModelApp's usage of it.

EDIT: looking at @bentley/imodeljs-i18n::I18n's constructor it seems to create a unique i18n instance so I think I would need to duplicate the initialization to translate before, unless/until itwin-viewer gets support for passing a custom I18n instance

EDIT2: looking at another app's code which initializes its own i18n explicitly, it seems like their approach is to just have both instances co-exist, and just use react-i18next to for their application-specific (non-imodeljs) translations, while imodeljs uses its own. Instead they store the data in the initialization of the static i18n. I will just go with that approach.

MichaelBelousov commented 4 years ago

Closing and will register reducers myself as @aruniverse suggested.