Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Async lookup for sidenav instance logs an error in the console (when it should just wait until the instance is available). #8308

Closed
yoav-zibin opened this issue May 2, 2016 · 6 comments
Assignees
Labels
has: Pull Request A PR has been created to address this issue type: bug
Milestone

Comments

@yoav-zibin
Copy link

yoav-zibin commented May 2, 2016

From the docs:
https://material.angularjs.org/latest/api/service/$mdSidenav

// Async lookup for sidenav instance; will resolve when the instance is available
$mdSidenav(componentId).then(function(instance) {
$log.debug( componentId + "is now ready" );
});

However, when I do that, I get an error in the logs:

angular.js:13550 SideNav 'left' is not available! Did you use md-component-id='left'?
(anonymous function) @ angular.js:13550findInstance @ angular-material.js:15747(anonymous function) @ angular-material.js:15712(anonymous function) @ index.dev.html:520invoke @ angular.js:4665(anonymous function) @ angular.js:4473forEach @ angular.js:322createInjector @ angular.js:4473doBootstrap @ angular.js:1746bootstrap @ angular.js:1767gamingPlatformInitFinished @ index.dev.html:530(anonymous function) @ app-gameinvite-common-code.ts:258trigger @ angular.js:3166defaultHandlerWrapper @ angular.js:3456eventHandler @ angular.js:3444

Even when passing true, it still logs the error:
$mdSidenav('left', true);

Except the error logs, things work correctly: the promise is resolved correctly :)
The problem is that it spams the error logs (and i have protractor tests that make sure the error log is empty).

The problem is in this code:

/**
   * Service API that supports three (3) usages:
   *   $mdSidenav().find("left")                       // sync (must already exist) or returns undefined
   *   $mdSidenav("left").toggle();                    // sync (must already exist) or returns reject promise;
   *   $mdSidenav("left",true).then( function(left){   // async returns instance when available
   *    left.toggle();
   *   });
   */
  return function(handle, enableWait) {
    if ( angular.isUndefined(handle) ) return service;

    var instance = service.find(handle); // THIS IS THE BUG: if enableWait is true, then it should use waitForInstance (and not findInstance).


Because findInstance logs an error if the instance is not ready:
function findInstance(handle) {
      var instance = $mdComponentRegistry.get(handle);
      if(!instance) {
        // Report missing instance
        $log.error( $mdUtil.supplant(errorMsg, [handle || ""]) );

See:

   var service = {
        find    : findInstance,     //  sync  - returns proxy API
        waitFor : waitForInstance   //  async - returns promise
      };
@yoav-zibin
Copy link
Author

yoav-zibin commented May 2, 2016

BTW, the documentation is buggy as it should mention the boolean argument (true):
$mdSidenav(componentId, true).then(function(instance) {

The workaround I found to avoid the log.error is to use [the undocumented] $mdComponentRegistry as follows:

$mdComponentRegistry.when('left').then(function() {
  // Now you can use $mdSidenav('left') or $mdSidenav('left', true) without getting an error.
  $mdSidenav('left').toggle();
})

@ThomasBurleson ThomasBurleson added this to the 1.1.0 milestone May 2, 2016
@crisbeto crisbeto added the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label May 3, 2016
@crisbeto
Copy link
Member

crisbeto commented May 3, 2016

This happens because firing the $mdSidenav immediately in your controller is earlier than the directive controller which registers it with the $mdComponentRegistry. I'll try to see if there's a workaround on Material's side, but in your case wrapping it in a $timeout should do the trick.

@yoav-zibin
Copy link
Author

I initially tried $timeout, but a low timeout (say even 200 milliseconds) wasn't enough to solve it --- maybe my macbook pro is slow :D

Then I found a better workaround using:
$mdComponentRegistry.when('left')

I believe this is what material should do:

return function(handle, enableWait) {
if ( angular.isUndefined(handle) ) return service;

if (enableWait) return waitForInstance(handle); // which calls $mdComponentRegistry.when(handle);

Simple :)

@crisbeto
Copy link
Member

crisbeto commented May 3, 2016

Yeah, passing in true does the same behind the scenes. We shouldn't throw errors in that case though.

@yoav-zibin
Copy link
Author

The current code doesn't do the same because it first tries to find the instance (which logs an error).
Note that it doesn't throw an error (just console.error):
$log.error( $mdUtil.supplant(errorMsg, [handle || ""]) );

Again, I believe the fix is a one line change:

Current code:
return function(handle, enableWait) {
if ( angular.isUndefined(handle) ) return service;

var instance = service.find(handle);
return  !instance && (enableWait === true) ? service.waitFor(handle) :
        !instance && angular.isUndefined(enableWait) ? addLegacyAPI(service, handle) : instance;

};

New code:

return function(handle, enableWait) {
if ( angular.isUndefined(handle) ) return service;
if (enableWait) return waitForInstance(handle); // NEW LINE
var instance = service.find(handle);
// I also simplified the line below
return !instance && angular.isUndefined(enableWait) ? addLegacyAPI(service, handle) : instance;
};

crisbeto added a commit to crisbeto/material that referenced this issue May 3, 2016
* The `$mdSidenav` service was throwing errors, even if the option to wait for an asynchronous instance was passed. This changes it to only log if the lookup for the instance is synchronous.
* Fixes the error for an invalid sidenav instance name not being logged if the `enableWait` argument is passed.
* Adds info about the async argument of `$mdSidenav` in the docs.

Fixes angular#8308.
@crisbeto crisbeto added has: Pull Request A PR has been created to address this issue and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs labels May 3, 2016
crisbeto added a commit to crisbeto/material that referenced this issue May 4, 2016
* The `$mdSidenav` service was logging errors, even if the option to wait for an asynchronous instance was passed. This changes it to only log if the lookup for the instance is synchronous.
* Fixes the error for an invalid sidenav instance name not being logged if the `enableWait` argument is passed.
* Adds info about the async argument of `$mdSidenav` in the docs.

Fixes angular#8308.
@davasqueza
Copy link

davasqueza commented Oct 2, 2017

I have the same issue, with the workarround of @yoav-zibin the sidernav works well, but in my case if you navigate to another route and go back the sidernav stop working without any warning or error log, I fixed that destroying the sidernav on scope destroy event.

It's strange, but I really don't sure why this works.

var sidenavPromise = $mdComponentRegistry.when('sidenav');
var sidenavInstance;

sidenavPromise.then(function() {
      if(sidenavInstance){
        sidenavInstance.destroy();
      }
      sidenavInstance= $mdSidenav("sidenav");
      $scope.$on("$destroy", sidenavInstance.destroy);
     //Sidenav logic
});

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
has: Pull Request A PR has been created to address this issue type: bug
Projects
None yet
Development

No branches or pull requests

4 participants