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

Pipes break when used in a ng-switch-default #5169

Closed
Xesued opened this issue Nov 6, 2015 · 21 comments
Closed

Pipes break when used in a ng-switch-default #5169

Xesued opened this issue Nov 6, 2015 · 21 comments
Labels
area: common Issues related to APIs in the @angular/common package type: bug/fix

Comments

@Xesued
Copy link

Xesued commented Nov 6, 2015

When using a custom pipe inside of an ng-switch-default the pipe breaks with the exception:

EXCEPTION: TypeError: Cannot read property 'constructor' of undefined in [null]

TypeError: Cannot read property 'constructor' of undefined
    at Object.implementsOnDestroy (https://code.angularjs.org/2.0.0-alpha.45/angular2.dev.js:7655:16)
    at Function.ChangeDetectionUtil.callPipeOnDestroy (https://code.angularjs.org/2.0.0-alpha.45/angular2.dev.js:16480:38)
    at AbstractChangeDetector.ChangeDetector_Hello_5.dehydrateDirectives (eval at <anonymous> (https://code.angularjs.org/2.0.0-alpha.45/angular2.dev.js:20571:14), <anonymous>:93:49)
    at AbstractChangeDetector.dehydrate (https://code.angularjs.org/2.0.0-alpha.45/angular2.dev.js:20384:12)
    at AppViewManagerUtils.dehydrateView (https://code.angularjs.org/2.0.0-alpha.45/angular2.dev.js:17481:35)
    at AppViewManager_._viewDehydrateRecurse (https://code.angularjs.org/2.0.0-alpha.45/angular2.dev.js:21117:21)
    at AppViewManager_._destroyViewInContainer (https://code.angularjs.org/2.0.0-alpha.45/angular2.dev.js:21105:12)
    at AppViewManager_.destroyViewInContainer (https://code.angularjs.org/2.0.0-alpha.45/angular2.dev.js:21060:12)
    at ViewContainerRef_.remove (https://code.angularjs.org/2.0.0-alpha.45/angular2.dev.js:9707:24)
    at ViewContainerRef_.ViewContainerRef.clear (https://code.angularjs.org/2.0.0-alpha.45/angular2.dev.js:9632:14)BrowserDomAdapter.logError @ angular2.dev.js:21984ExceptionHandler.call @ angular2.dev.js:4439(anonymous function) @ angular2.dev.js:19685NgZone._notifyOnError @ angular2.dev.js:10746errorHandling.onError @ angular2.dev.js:10654run @ angular2.dev.js:141(anonymous function) @ angular2.dev.js:10678zoneBoundFn @ angular2.dev.js:111lib$es6$promise$asap$$flush @ angular2.dev.js:1301
2015-11-06 15:24:17.011 angular2.dev.js:21984 ERROR CONTEXT:BrowserDomAdapter.logError @ angular2.dev.js:21984ExceptionHandler.call @ angular2.dev.js:4442(anonymous function) @ angular2.dev.js:19685NgZone._notifyOnError @ angular2.dev.js:10746errorHandling.onError @ angular2.dev.js:10654run @ angular2.dev.js:141(anonymous function) @ angular2.dev.js:10678zoneBoundFn @ angular2.dev.js:111lib$es6$promise$asap$$flush @ angular2.dev.js:1301

Here is a plunker: http://plnkr.co/edit/PSRVVPtbl6MzRzNPzc2b?p=preview
When I remove the ng-switch-default template, I don't get the error.

Using Chrome 46 on Mac.

@Ristaaf
Copy link

Ristaaf commented Dec 1, 2015

Same kind of issue (exactly the same callstack) if having an expression with an optional pipe, like this:
"true===true || 'Hello World' | myPipe"
Since myPipe will never be called in this example, it will never be instantiated, but when I navigate away from the page, and all used pipes is supposed to be destroyed, it will try to destroy myPipe too, even if it has not been used, and I get the error.

@scotthowl
Copy link

I'm seeing the same exception reported by Xesued when I use a pipe inside of an ng-switch-default. I'm developing with angular 2.0.0-alpha.46 using chrome 47.0.2526.73 on a windows 10 machine. His plunker reproduces the error perfectly.

@sumigoma
Copy link

sumigoma commented Dec 8, 2015

+1

@jimjim2a
Copy link

+1 same as Xesued

KillerCodeMonkey added a commit to angularjs-de/angular2-pizza-service that referenced this issue Jan 9, 2016
@KillerCodeMonkey
Copy link

+1 it is still there *beta.1

@Matthias247
Copy link

The issue also occurs with builtin pipes like the JSON pipe.
Besides leading to a runtime issue in the pipe it will also lead to the all other content of the ngSwitchDefault clause to be displayed when the clause doesn't match and an ngSwitchWhen clause matches.

Here's a plnkr from me for reproduction:
http://plnkr.co/edit/2YzXobiF7IBOeODK1tST
Removing the | json part in ngSwitchDefault makes it work correctly.

@danielfigueiredo
Copy link

+1

1 similar comment
@adammolnar
Copy link

+1

@KiaraGrouwstra
Copy link
Contributor

I simplified @Xesued's reproduction -- no custom pipe appears needed, just ensure ngFor keeps an unused branch so it tries to destroy a pipe that didn't get initialized.
I wasn't able to reproduce with @Ristaaf's optional pipe though.
I'm a bit confused though, since in @Xesued's example all ngFor branches do get used by different entries... but the common thread seems sorta clear.

So the crash occurs here, due to pipe ending up undefined. I think this could be patched as follows:

function implementsOnDestroy(pipe: any): boolean {
    // return pipe.constructor.prototype.ngOnDestroy;
    return pipe ? pipe.constructor.prototype.ngOnDestroy : false;
}

(For debugging I also tried to add a print statement in the parent stack level; here selectedPipe turned out as {}:)

    ChangeDetectionUtil.callPipeOnDestroy = function (selectedPipe) {
        console.log('callPipeOnDestroy:selectedPipe', selectedPipe);
        if (pipe_lifecycle_reflector_1.implementsOnDestroy(selectedPipe.pipe)) {
            selectedPipe.pipe.ngOnDestroy();
        }
    };

I'm having trouble locally running the tests to verify this null check fixes the issue without messing up other things though; perhaps someone else would be able to try that...

Edit: to clarify, I haven't verified if suppressing the error makes things work as desired; on my own code I still had an unrelated error I was already getting on other code as well, while with Plunker I'm not sure I can just alter the ng2 code like this, so I haven't actually seen the result.

@KiaraGrouwstra
Copy link
Contributor

Having found a workaround for that issue now, I'm now able to confirm that the above check does indeed appear to address this issue.

I wouldn't know what the right module for this should be, but I believe a test for the above fix could potentially look as follows.

import { Component } from 'angular2/core';
import { NgSwitch, NgSwitchWhen, NgSwitchDefault, JsonPipe } from 'angular2/common';

@Component({
  selector: 'issue_5169',
  directives: [NgSwitch, NgSwitchWhen, NgSwitchDefault, JsonPipe],
  template: `
    <div [ng-switch]="val">
    <template [ng-switch-when]="1">one</template>
    <template ng-switch-default>{{val | json}}</template>
    </div>
    `
})
export class SwitchPipeComp {
  constructor(){
    this.val = 1;
  }
}
  it('should consume element binding changes',
    inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
      tcb.createAsync(SwitchPipeComp).then((fixture) => {
        // crashed: [#5169](https://github.com/angular/angular/issues/5169)
        expect(fixture.debugElement).toHaveText('one');
        async.done();
      });
  }));

... I'd appreciate further help though.

@teleaziz
Copy link
Contributor

teleaziz commented Mar 2, 2016

+1
Same issue occurs everytime when I use a pipe inside an *ngIf with a condition the didn't get triggered

@boban100janovski
Copy link

+1
Yep happens if a pipe is inside an *ngIf that is falsy

@jakemoves
Copy link

+1 Confirming this issue.

@kerlin0800
Copy link

+1
Also I have this issue. When you fix this?

@laurelnaiad
Copy link

suffering this bug?

import this before you import angular stuff:

"format register";
interface System {
  register: Function
}

System.register("angular2/src/core/change_detection/pipe_lifecycle_reflector", [], true, function(require, exports, module) {
  exports.implementsOnDestroy = (pipe) => pipe ? pipe.constructor.prototype.ngOnDestroy : false
})

Obviously this is the hack from above, just packaged so that you can apply it without hacking into angular's files.

@hadarge
Copy link

hadarge commented Apr 13, 2016

+1 same with default router :(

@damiandennis
Copy link

@laurelnaiad Tried your work around but could not get it to work.

Also tried in bootstrap.ts with format: register in my system-config.js

declare var System: {
    register: Function
};
System.register("angular2/src/core/change_detection/pipe_lifecycle_reflector", [], true, function(require, exports, module) {
    exports.implementsOnDestroy = (pipe) => pipe ? pipe.constructor.prototype.ngOnDestroy : false
});

@damiandennis
Copy link

@laurelnaiad Figured it out

<script src="node_modules/systemjs/dist/system.src.js"></script>
    <script>
        System.register("angular2/src/core/change_detection/pipe_lifecycle_reflector", [], true, function(require, exports, module) {
            exports.implementsOnDestroy = function(pipe) { return pipe ? pipe.constructor.prototype.ngOnDestroy : false };
        });
    </script>

This did it for me.

@KiaraGrouwstra
Copy link
Contributor

The file in question no longer exists in RC. Is this issue still relevant there? I failed to reproduce.

@mhevery mhevery added area: common Issues related to APIs in the @angular/common package and removed comp: core/pipes labels Sep 7, 2016
@pkozlowski-opensource
Copy link
Member

Can no longer reproduce: https://plnkr.co/edit/E662nxksiK4z10gJAv7y?p=preview

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: common Issues related to APIs in the @angular/common package type: bug/fix
Projects
None yet
Development

No branches or pull requests