Revert async loading components in ide.vue
Revert async loading components in ide.vue- Defer loading terminal modules in Web IDE
- Add docs for potential test timeout issue
What does this MR do?
This MR fixes an issue with Jest tests timing out #280809 (closed) because the component modules are being compiled at runtime which was caused by deferring their import over here !47029 (merged).
It fixes this by reverting the async loading for just synchronous loading.
Why prefer sync loading the components here?
1 - Async loading has no compile-time safety
If we typo an import path, neither webpack nor jest loaders complain
diff --git a/app/assets/javascripts/ide/components/ide.vue b/app/assets/javascripts/ide/components/ide.vue
index e1d2895831a..07b0c817a79 100644
--- a/app/assets/javascripts/ide/components/ide.vue
+++ b/app/assets/javascripts/ide/components/ide.vue
@@ -37,7 +37,7 @@ export default {
components: {
IdeSidebar,
RepoEditor,
- 'error-message': () => import('./error_message.vue'),
+ 'error-message': () => import('./error_message_bogus.vue'),
'gl-button': () => import('@gitlab/ui/src/components/base/button/button.vue'),
'gl-loading-icon': () => import('@gitlab/ui/src/components/base/loading_icon/loading_icon.vue'),
'commit-editor-header': () => import('./commit_sidebar/editor_header.vue'),
2 - Async loading can cause Jest specs to potentially time out
There's an unsolved problem with the relationship between our production code and the jest runtime. If a component is imported dynamically, Jest will need to transform it on the spot, which happens during runtime and potentially causes it to time out like in #280809 (closed).
You'll notice at the time of writing this that the ide component
spec has the slowest test.
3 - The speed performance benefit of async loading these components is questionable
The following values are the measurement of the webide-tree-loading-from-request
performance measurement when hitting /-/ide/project/gnuwget/wget2/edit/master/-/
locally.
Run 1
is on a fresh webpack
after running gdk restart webpack
and refreshing the page once.
Measure | Run 1 | Run 2 | Run 3 | Run 4 | Run 5 | Average |
---|---|---|---|---|---|---|
this MR | 9.21s | 8.84s | 8.95s | 8.80s | 9.24s | 9.008s |
on master
|
8.83s | 8.77s | 8.86s | 9.34s | 9.26s | 9.012s |
This is an incredibly small and imperfect statistical sample, so the only thing we can conclude here is that we can't conclude there is a difference.
4 - The code-splitting performance of async loading these components is questionable
The following files are logs of requests made to JS files when hitting /-/ide/project/gnuwget/wget2/edit/master/-/
locally on master
and this MR (respectively):
Measure | File | Total Request Count | Total Size in KB |
---|---|---|---|
this MR | gitlab.requests.sync_components.json | 14 | 39,716 KB |
on master
|
gitlab.requests.master.json | 19 | 39,559 KB |
If you break down the differences between these two logs master
(which currently loads these components async) adds about 7 more chunks totaling to approximately 2,050 KB. The difference between the pages.ide.chunk.js
in this MR and master
is approximately 1,140 KB. So where is the rest coming from?
It turns out that this MR (which loads the components synchronously) causes two extra chunks to be loaded, one which is noticeably bigger commons-pages.ide-pages.projects.environments.terminal-pages.projects.jobs.terminal.chunk.js
.
If we just target defering this chunk until it's actually needed (like in !47719 (merged)), then we can end up with a much favorable code-splitting situation.
Measure | File | Total Request Count | Total Size in KB |
---|---|---|---|
follow up MR !47719 (merged) | gitlab.requests.sync_components_except_terminal.json | 13 | 38,528 KB |
Screenshots (strongly suggested)
It still works
References
- Related to #280809 (closed)
- Relevant MR !47029 (merged)