Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

appContent is undefined for pre-loaded apps #211

Closed
rocco1007 opened this issue Dec 19, 2014 · 8 comments
Closed

appContent is undefined for pre-loaded apps #211

rocco1007 opened this issue Dec 19, 2014 · 8 comments
Milestone

Comments

@rocco1007
Copy link

I am working on pre-loading apps and getting an error that "appContent" is undefined when the App_Class is loaded. After taking a look at f2.debug.js it doesn't look like appContent gets passed to _createAppInstance in registerApps. Is there a way to manually pass this and/or can F2 be updated to pass this?

here is a snippet of the code roughly line 16539:

// add properties and methods
a = _createAppConfig(a);

// Will set to itself, for preloaded apps, or set to null for apps that aren't already
// on the page.
a.root = a.root || null;

// we validate the app after setting the root property because pre-load apps do no require
// manifest url
if (!_validateApp(a)) {                                 
        return; // move to the next app
}

// save app
_apps[a.instanceId] = { config:a };

// If the root property is defined then this app is considered to be preloaded and we will
// run it through that logic.
if(a.root)
{
if((!a.root && typeof(a.root) != 'string') && !F2.isNativeDOMNode(a.root))
{
                F2.log('AppConfig invalid for pre-load, not a valid string and not dom node');
                F2.log('AppConfig instance:', a);
                throw('Preloaded appConfig.root property must be a native dom node or a string representing a sizzle selector. Please check your inputs and try again.');
}
else if(jQuery(a.root).length != 1)
{
                F2.log('AppConfig invalid for pre-load, root not unique');
                F2.log('AppConfig instance:', a);
                F2.log('Number of dom node instances:', jQuery(a.root).length);
                throw('Preloaded appConfig.root property must map to a unique dom node. Please check your inputs and try again.');
}

// instantiate F2.App
_createAppInstance(a);

// init events
_initAppEvents(a);

// Continue on in the .each loop, no need to continue because the app is on the page
// the js in initialized, and it is ready to role.
return; // equivalent to continue in .each
}
markhealey pushed a commit that referenced this issue Dec 22, 2014
@markhealey
Copy link
Member

@rocco1007 I added a unit test for this because we didn't have one already. I attached a screenshot of the output and there are 3 arguments being passed to the AppClass as intended including appContent.

appcontent-test

So, can you reproduce this in a jsFiddle?

@rocco1007
Copy link
Author

@markhealey thanks for looking into this. Here is the jsFiddle. I pretty much used the example from the Pre-Loading docs here. Also, below is a screenshot.

appcontent

@rocco1007
Copy link
Author

Hi All,

Has anyone had a chance to look into this issue?

@montlebalm
Copy link
Member

@markhealey The test only looks for the number of arguments, not that they're correct. Rocco's fiddle shows that the second param, "appContent", is undefined.

Are there conditions in which "appContent" should be undefined? If not we should look into what's causing it and change the test to be more specific.

@markhealey
Copy link
Member

Excellent fiddle @rocco1007, thanks. Sorry for the delayed look at this, holidays and all that sort of thing.

@montlebalm I can't think of a single reason that appContent should be undefined. At the very least it should be an empty string.

@markhealey
Copy link
Member

@montlebalm did you end up doing any work on this one?

@montlebalm
Copy link
Member

@markhealey only enough to confirm the problem.

@markhealey markhealey added this to the 1.4.1 milestone Apr 20, 2015
This was referenced Apr 23, 2015
@qrider
Copy link
Contributor

qrider commented Apr 15, 2016

patch verified in 1.4.1-wip here => http://plnkr.co/edit/XM0LYh?p=preview

@qrider qrider closed this as completed Apr 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants