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

“Expression has changed after it was checked” #84 #111

Merged
merged 1 commit into from
Nov 10, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions src/time-ago.pipe.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import 'es6-shim';
import 'reflect-metadata';
import {NgZone} from '@angular/core';
import {TimeAgoPipe} from './time-ago.pipe';
import * as moment from 'moment';

// under systemjs, moment is actually exported as the default export, so we account for that
const momentConstructor: (value?: any) => moment.Moment = (<any>moment).default || moment;

class NgZoneMock {
runOutsideAngular (fn: Function) {
Expand Down Expand Up @@ -38,6 +42,31 @@ describe('TimeAgoPipe', () => {
expect(changeDetectorMock.markForCheck).toHaveBeenCalled();
});

it('should update the text with a new date instance different from the previous one', () => {
const changeDetectorMock = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck']);
const pipe = new TimeAgoPipe(changeDetectorMock, new NgZoneMock() as NgZone);
jasmine.clock().mockDate(new Date('2016-01-01'));
expect(pipe.transform(new Date())).toBe('a few seconds ago');
expect(pipe.transform(new Date(0))).toBe('46 years ago');
expect(pipe.transform(moment())).toBe('a few seconds ago');
expect(pipe.transform(moment(0))).toBe('46 years ago');
});

it('should update the text when the date instance time is updated', () => {
const changeDetectorMock = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck']);
const pipe = new TimeAgoPipe(changeDetectorMock, new NgZoneMock() as NgZone);
jasmine.clock().mockDate(new Date('2016-01-01'));
let date = new Date();
expect(pipe.transform(date)).toBe('a few seconds ago');
date.setFullYear(2000);
expect(pipe.transform(date)).toBe('16 years ago');

let dateAsMoment = moment();
expect(pipe.transform(dateAsMoment)).toBe('a few seconds ago');
dateAsMoment.year(2000);
expect(pipe.transform(dateAsMoment)).toBe('16 years ago');
});

it('should remove all timer when destroyed', () => {
const changeDetectorMock = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck']);
const pipe = new TimeAgoPipe(changeDetectorMock, new NgZoneMock() as NgZone);
Expand Down
56 changes: 49 additions & 7 deletions src/time-ago.pipe.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,63 @@
/* angular2-moment (c) 2015, 2016 Uri Shaked / MIT Licence */

import { Pipe, ChangeDetectorRef, PipeTransform, OnDestroy, NgZone } from '@angular/core';
import {Pipe, ChangeDetectorRef, PipeTransform, OnDestroy, NgZone} from '@angular/core';
import * as moment from 'moment';

// under systemjs, moment is actually exported as the default export, so we account for that
const momentConstructor: (value?: any) => moment.Moment = (<any>moment).default || moment;

@Pipe({ name: 'amTimeAgo', pure: false })
@Pipe({name: 'amTimeAgo', pure: false})
export class TimeAgoPipe implements PipeTransform, OnDestroy {
private currentTimer: number;

private lastTime: Number;
private lastValue: Date | moment.Moment;
private lastOmitSuffix: boolean;
private lastText: string;

constructor(private cdRef: ChangeDetectorRef, private ngZone: NgZone) {
}

transform(value: Date | moment.Moment, omitSuffix?: boolean): string {
const momentInstance = momentConstructor(value);
if (this.hasChanged(value, omitSuffix)) {
this.lastTime = this.getTime(value);
this.lastValue = value;
this.lastOmitSuffix = omitSuffix;
this.removeTimer();
this.createTimer();
this.lastText = momentConstructor(value).from(momentConstructor(), omitSuffix);

} else {
this.createTimer();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of calling createTimer() here? If we had a previous value, we have already created the timer, no?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its purpose is to recreate the timer once its callback has ran.

When the callback is running, lastText is computed and markForCheck is called. The timer must be recreated. Instead of resubmitting the timer inside the callback timer which could lead to more than one markForCheck for a given transform, I recreate the timer in the transform method because a markForCheck will lead to a transform calls.

Hope I'm understandable.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks!

}

return this.lastText;
}

ngOnDestroy(): void {
this.removeTimer();
}


private createTimer() {
if (this.currentTimer) {
return;
}
const momentInstance = momentConstructor(this.lastValue);

const timeToUpdate = this.getSecondsUntilUpdate(momentInstance) * 1000;
this.currentTimer = this.ngZone.runOutsideAngular(() => {
if (typeof window !== 'undefined') {
return window.setTimeout(() => {
this.lastText = momentConstructor(this.lastValue).from(momentConstructor(), this.lastOmitSuffix);

this.currentTimer = null;
this.ngZone.run(() => this.cdRef.markForCheck());
}, timeToUpdate);
}
});
return momentConstructor(value).from(momentConstructor(), omitSuffix);
}

ngOnDestroy(): void {
this.removeTimer();
}

private removeTimer() {
if (this.currentTimer) {
Expand All @@ -50,4 +78,18 @@ export class TimeAgoPipe implements PipeTransform, OnDestroy {
return 3600;
}
}

private hasChanged(value: Date | moment.Moment, omitSuffix?: boolean) {
return this.getTime(value) !== this.lastTime || omitSuffix !== this.lastOmitSuffix;
}

private getTime(value: Date | moment.Moment) {
if (moment.isDate(value)) {
return value.getTime();
} else if (moment.isMoment(value)) {
return value.valueOf();
} else {
return momentConstructor(value).valueOf();
}
}
}