Move main.js to modules and help tab #2430
Conversation
chmelevskij
changed the title
| break; | ||
| case 'help': | ||
| TABS.help.initialize(content_ready); | ||
| import('./tabs/help').then(({ help }) => help.initialize(content_ready)); |
There was a problem hiding this comment.
Idea is to move the rest of the tabs to this usage. This way every tab and it's dependencies will be migrated one by one. Eventually leaving single main.js as entry point and removing the global variables.
There was a problem hiding this comment.
Ok. It seems a little strange to me but I don't have experience enough to know if this is the best way or not.
There was a problem hiding this comment.
I've tried few different approaches. All of them are hard, because there was no clear separation on how to do it incrementally. This approach with TABS being the separation point is the most straightforwards.
we could definitely change the way tabs are exported and handled. That will be the next step. Didn't wan't to change a that because there are so many tabs already, and testing/reviewing a lot of changes will be hard enough already, there are a lot of other small things which have to be resolved. Like circular dependencies and random global variables.
There was a problem hiding this comment.
The approach to do the conversion step-by-step in this way seems reasonable.
What is the reason you chose to import the tabs here, instead of importing all of them at the top of the file, and just initialising them here?
McGiverGim
left a comment
McGiverGim
left a comment
There was a problem hiding this comment.
It seems a correct approach to me. Only one suggestion to discuss about moving to the end of the file the window declarations.
| break; | ||
| case 'help': | ||
| TABS.help.initialize(content_ready); | ||
| import('./tabs/help').then(({ help }) => help.initialize(content_ready)); |
There was a problem hiding this comment.
Ok. It seems a little strange to me but I don't have experience enough to know if this is the best way or not.
|
Kudos, SonarCloud Quality Gate passed!
|
| break; | ||
| case 'help': | ||
| TABS.help.initialize(content_ready); | ||
| import('./tabs/help').then(({ help }) => help.initialize(content_ready)); |
There was a problem hiding this comment.
The approach to do the conversion step-by-step in this way seems reasonable.
What is the reason you chose to import the tabs here, instead of importing all of them at the top of the file, and just initialising them here?
Could probably go that route as well tbh. Usually this would improve the performance a bit, since we wouldn't load all the js, but it's a desktop app. But even for desktop app, I think it would make more sense to initialize and load them dynamically. rather than loading all of the code all at once. |
|
Fair enough, we are trading off maintainability for loading time here. |
It's not like it got worse 😆 |
That's exactly how you end up with codebases that are rubbish in the first place... |
3 participants
Moving more parts to esm. Jus realised that
main.jsworks with all the global stuff. So idea is to move it first and then move on tab basis