OpenF2 / F2

Redefining web integration for the financial services community
Apache License 2.0
129 stars 62 forks source link

Exception thrown when root element is removed from DOM before / during appRender #194

Open md-jafuss opened 9 years ago

md-jafuss commented 9 years ago

Our client hosts the F2 Container for the apps we have created for one of our projects and they have a structure setup so that they can remove elements off the DOM and add them back to the DOM when the tab for that content becomes the active view.

This page is a single page application so there isn't any full page refresh unless the user does this them self.

The issue they are running into is that an exception is thrown stating that their root element for the apps they pre-render are not in the DOM. The specific error is thrown in the following line of code (line 421) showing up in sdk\src\container.js (they are using the f2.min.js):

if (!elementInDocument(root)) { throw ('App root for ' + appId + ' was not appended to the DOM. Check your AppHandler callbacks to ensure you have rendered the app root to the DOM.'); }

I did a quick test by removing this if statement, saved, then at the end of my appRender I stored the root element in a JavaScript object and removed the element from the DOM, and then it proceeded through the rest of the F2 code. After the page was done loading I added the root element I saved in a JavaScript object back into the container it would have originally rendered and then tested that all the JavaScript and CSS was working / showing correctly.

It seemed to work as expected from what I could see so I was just wondering what the purpose of this if(!elementInDocument(root)) check is for and whether it is necessary for something to work properly in the F2 codebase?

Based on my small test I didn't see any issues but I could be missing something that is used in a more complicated scenario. Please let me know if you need additional details to investigate or reply to this.

brianbaker commented 9 years ago

@montlebalm can you recall if there was a specific use case for the elementInDocument check here? Was that already in there before your updates to allow overriding for loading scripts and styles?

markhealey commented 9 years ago

cc @Ali-Khatami

montlebalm commented 9 years ago

@brianbaker The check for root existed before me, but I refactored it. I believe it's there to make sure the container developer correctly implemented their app handlers.

Ali-Khatami commented 9 years ago

@fusseneg I think I'm being thrown off by,

The issue they are running into is that an exception is thrown stating that their root element for the apps they pre-render are not in the DOM.

There is a concept in F2 of PreRendered apps from the serverside that are already on the page, but need to be initialized in F2. The goal was to allow developers to load apps via the server, render the page in its entirety avoiding loading spinners and fragmented load.

I'll have to look back through the logs to figure out what the check was prior to @montlebalm's changes. In the mean time is there anyway to create a fiddle showing the issue without revealing sensitive information?

md-jafuss commented 9 years ago

@Ali-Khatami Is this pre-loading apps possible if we do not host the F2 container? Also since their page is a single page app (no page refesh) when they enter a symbol to load the symbol specific sections it would load their apps (non-f2) as well as our apps so I feel I would need to understand the pre-loading process a bit before going with this solution.

They are attempting to make a request to load apps that may not have a root element in the DOM yet (but stored in javascript object) so they can load apps in other sections while the user is viewing the current visible section.

Also our client noticed that some apps will take much longer than others so they ended up turning off the batching request for majority of the apps shown within their page since it was causing performance issues trying to load all the apps in the same batch.

If the pre-loading has better performance than we can suggest this solution but due to time constraints this would have to be a longer term solution. Feel free to stop by if you would like to see the site to get more details.

md-jafuss commented 9 years ago

@Ali-Khatami As for answering your question.

If you were to create a fiddle it would essentially just be setting up an appRender that creates some app containers that are stored in a JavaScript object but don't get appended to the DOM. Then you could add a link or button you click to append one of these containers (or all) to the DOM.