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

issue after merged all js resources with r.js in a requirejs project #18

Closed
nebulaszlonemethi opened this issue Apr 23, 2014 · 6 comments

Comments

@nebulaszlonemethi
Copy link

Hi,
let me describe an issue i ran into: when i merge all js files with r.js in an Angular + ui-router + requirejs + ocLazyLoad project and try to use it without any config modifications, however the merged js is syntactically correct, ocLazyLoad.load() will incorrectly load new modules.

Problem: because the "new" module to be loaded is already defined in the merged file with all its components, the moduleExists() returns true so ocLazyLoad.load() WON'T call the register() method for that, so calling the module's components will fail.

Using the non-merged files, the issue doesn't exist cus the moduleExists() will return false so ocLazyLoad.load() will correctly load+register the new module.

(my quick workaround was, insert the line
register(providers, moduleCache, $log);
right after line 101, after row
moduleCache.push(moduleName);)

very useful plugin btw, thanks for it!

@ocombe
Copy link
Owner

ocombe commented Apr 23, 2014

Hmm I think I understand the problem yes, I'm not sure how to fix it though.
Removing the moduleCache could lead to unexpected results with modules overwriting and config / run being launched twice and that sort of things...

@nebulaszlonemethi
Copy link
Author

The moduleCache should not be removed, my "workaround" was only an 5-minutes experiment after meeting the bug:).

What about that? (~line 100):

if (moduleExists(moduleName) && regModules[moduleName]) {
deferred.resolve();
return deferred.promise;
}

instead of:

if (moduleExists(moduleName)) {
moduleCache.push(moduleName);
deferred.resolve();
return deferred.promise;
}

This way a new module with its dependencies will be passed to the register() anyway unless it has already been properly loaded (=registered in the regModules array). The register() method seems like properly avoids from the re-launching by checking the regModules array. Also of course, ocLazyLoad should be properly configured initially through its provider's loadedModules property in the app config().

@nebulaszlonemethi
Copy link
Author

sorry the code is not correct (regModules is not a hashmap), i've meant:
if (moduleExists(moduleName) && regModules.indexOf(moduleName])>=0) {
deferred.resolve();
return deferred.promise;
}

@ocombe
Copy link
Owner

ocombe commented Apr 30, 2014

Ok nice catch, this should be fixed in the new release 0.1.3, could you check if it works fine for you now ?

@nebulaszlonemethi
Copy link
Author

Works perfectly, thanks! The issue can be closed. One really minor thing: i guess the line 101., right after the altered condition:
moduleCache.push(moduleName);
can be removed since moduleCache is a local variable defined in the function load(), makes no sense to modify it just before returning from the load().

@ocombe
Copy link
Owner

ocombe commented May 2, 2014

You're right, I'll do that in a future commit

@ocombe ocombe closed this as completed May 2, 2014
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

No branches or pull requests

2 participants