refactor: different config loading strategy #4807
Conversation
|
Probably should add tests for loadConfig action |
| const configUrl = getConfigUrl(); | ||
| const isPreloaded = preloadedConfig && preloadedConfig.size > 1; | ||
| const isPreloaded = preloadedConfig && Object.keys(preloadedConfig).length > 1; |
There was a problem hiding this comment.
Should it be rather Object.keys(preloadedConfig).length > 0?
|
I believe this PR should also fix the problem with fast refresh. Currently when cms is being fast-refreshed, config validation results in error: That's because after refresh, we validate existing config (which was normalized and to which defaults were applied), where each collection has view_filters and view_groups equal to empty array. |
6d4953c to
313c910
Compare
There was a problem hiding this comment.
Thanks again for this @smashercosmo, it's a tricky one, but the code looks much better now.
I added another commit with some cleanup and a fix to manual initialization.
Please let me know what you think.
|
thx @erezrokah. Much better with your changes. |
|
btw, did you have a chance to check if this fixes the issue with hot reload? |
Yes it does 🥳 |
* move config loading from App.js to bootstrap.js * remove mergeConfig action * introduce deepmerge package * fix: manual init Co-authored-by: erezrokah <erezrokah@users.noreply.github.com>
Summary
Fixes #4817
In this refactoring config loading has been moved from App.js to bootstrap.js, which makes App component "dumber". And also redundant mergeConfig and configDidLoad actions has been removed. This also should make #4776 merging easier.
Test plan
Run current tests