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

TruncatePipe fails with undefined or null input value to truncate #162

Closed
cdoremus opened this issue Nov 7, 2016 · 3 comments
Closed

TruncatePipe fails with undefined or null input value to truncate #162

cdoremus opened this issue Nov 7, 2016 · 3 comments

Comments

@cdoremus
Copy link
Contributor

cdoremus commented Nov 7, 2016

When value to truncate is undefined or null, the TruncatePipe.transform() call throws a TypeError. This error points to line 12 of truncate.pipe.ts where value.length is evaluated. See below for a spec to replicate this error.

Examining the source code of standard Angular 2 pipes that work with strings (e.g. lowercase or uppercase. see: https://github.com/angular/angular/blob/2.1.2/modules/angular/common/src/pipes/uppercase_pipe.ts), shows that the pipe transform() returns null in this case. However, another option is to have transform() return an empty string in the case when value is null or undefined. My opinion is to have TruncatePipe.transform() conform to the standard Angular 2 pipe behavior and return null when the value to truncate is null or undefined. Other opinions are welcome.

/**
 * Spec to replicate this issue
 */
import { Component } from '@angular/core';
import { TestBed } from '@angular/core/testing';

import { TruncatePipe } from './truncate.pipe';


describe('TruncatePipe', () => {
    let pipe;

    beforeEach(() => {
        TestBed.configureTestingModule({
            declarations: [TruncatePipe, TestComponent, Test2Component]
        })
        pipe = new TruncatePipe();
    });

    it('should return null if value is undefined', () => {
        let value = undefined;

        let actual = pipe.transform(value);

        expect(actual).toBeNull();
    });

    it('should return null if value is null', () => {
        let value = null;

        let actual = pipe.transform(value);

        expect(actual).toBeNull();
    });

    it('should return a null string inside a real component template when value to truncate is undefined', () => {
        // TestComponent defined below whose template uses the truncate pipe
        let fixture = TestBed.createComponent(TestComponent);
        let component = fixture.componentInstance;
        let message = component.message;
        let expected = '';
        fixture.detectChanges();

        let actual = fixture.nativeElement.querySelector('h2').innerText;

        expect(actual).toBeNull();
    });

    it('should return a null string inside a real component template when value to truncate is null', () => {
        // Test2Component defined below whose template uses the truncate pipe
        let fixture = TestBed.createComponent(TestComponent);
        let component = fixture.componentInstance;
        let message = component.message;
        fixture.detectChanges();

        let actual = fixture.nativeElement.querySelector('h2').innerText;

        expect(actual).toBeNull();
    });

});

@Component({
    selector: 'test',
    template: `<h2>{{ message | truncate }}</h2>`
})
class TestComponent {
    message = undefined;
}

@Component({
    selector: 'test2',
    template: `<h2>{{ message | truncate }}</h2>`
})
class Test2Component {
    message = null;
}
@lukad03
Copy link
Contributor

lukad03 commented Nov 7, 2016

@cdoremus Taking the Angular approach sounds like the right call. Would you be willing to make a PR with a fix? (Thanks for all of your other PRs so far - super helpful!)

@cdoremus
Copy link
Contributor Author

cdoremus commented Nov 7, 2016

Yes, I will create a PR to fix this issue including updating truncate.pipe.spec.ts.

cdoremus added a commit to cdoremus/code-gov-web that referenced this issue Nov 7, 2016
cdoremus added a commit to cdoremus/code-gov-web that referenced this issue Nov 7, 2016
lukad03 pushed a commit that referenced this issue Nov 8, 2016
Fix for null or undefined values to truncate defined in issue #162. Tests for this fix are included in truncate.pipe.spec.ts. Also cleaned up linting errors in the spec file.

* fix: added null check (issue #162)
* Added tests for null/undefined(#162). Fixed lint errors.
@lukad03
Copy link
Contributor

lukad03 commented Nov 8, 2016

Closed by #166

@lukad03 lukad03 closed this as completed Nov 8, 2016
BalajiJBcs pushed a commit to BalajiJBcs/code-gov-web that referenced this issue Jan 3, 2018
Fix for null or undefined values to truncate defined in issue GSA#162. Tests for this fix are included in truncate.pipe.spec.ts. Also cleaned up linting errors in the spec file.

* fix: added null check (issue GSA#162)
* Added tests for null/undefined(GSA#162). Fixed lint errors.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants