Skip to content

Commit

Permalink
fix: 🐛 issue when notifyOnChangeOnly stops emitting at all
Browse files Browse the repository at this point in the history
When a user provides notifyOnChangeOnly option, we are trying to compare
the new metadata with the old one by stringifying them. Turns out, that
JSON.stringify() does not work on Map, so it returns an empty object all
the time. Leading to broken comparsion that says that metadata never
changes.
  • Loading branch information
ghaiklor committed Sep 20, 2020
1 parent d03b379 commit e4700ca
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 5 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ module.exports = {
'max-len': ['error', 120],
'max-lines-per-function': ['off'],
'max-statements': ['off'],
'multiline-comment-style': ['error', 'separate-lines'],
'multiline-ternary': ['error', 'always-multiline'],
'no-ternary': ['off'],
'node/no-missing-import': ['error', { tryExtensions: ['.ts'] }],
Expand Down
2 changes: 1 addition & 1 deletion examples/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const radioStation = new Parser({
errorInterval: 10 * 60,
keepListen: false,
metadataInterval: 5,
notifyOnChangeOnly: false,
notifyOnChangeOnly: true,
url: 'https://live.hunter.fm/80s_high',
userAgent: 'Custom User Agent',
});
Expand Down
17 changes: 13 additions & 4 deletions src/Parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export declare interface Parser {
}

export class Parser extends EventEmitter {
private previousMetadata = '';
private previousMetadata: Map<string, string> = new Map<string, string>();
private readonly options: ParserOptions = {
autoUpdate: true,
emptyInterval: 5 * 60,
Expand Down Expand Up @@ -61,10 +61,9 @@ export class Parser extends EventEmitter {
this.destroyResponse(response);
this.queueNextRequest(this.options.metadataInterval);

const newMetadata = JSON.stringify(metadata);
if (this.options.notifyOnChangeOnly && newMetadata !== this.previousMetadata) {
if (this.options.notifyOnChangeOnly && this.isMetadataChanged(metadata)) {
this.previousMetadata = metadata;
this.emit('metadata', metadata);
this.previousMetadata = newMetadata;
} else if (!this.options.notifyOnChangeOnly) {
this.emit('metadata', metadata);
}
Expand Down Expand Up @@ -114,4 +113,14 @@ export class Parser extends EventEmitter {
protected queueRequest (timeout = 0): void {
setTimeout(this.makeRequest.bind(this), timeout * 1000);
}

protected isMetadataChanged (metadata: Map<string, string>): boolean {
for (const [key, value] of metadata.entries()) {
if (this.previousMetadata.get(key) !== value) {
return true;
}
}

return false;
}
}
11 changes: 11 additions & 0 deletions test/Parser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,15 @@ describe('parser', () => {
resolve();
});
}));

it('should properly emit metadata event when metadata has been updated', async () => await new Promise((resolve) => {
expect.hasAssertions();

const radio = new Parser({ autoUpdate: false, notifyOnChangeOnly: true, url: 'https://live.hunter.fm/80s_high' });
radio.on('metadata', (metadata) => {
// @ts-expect-error I want to check that metadata was stored in the private property to later comparsion
expect(radio.previousMetadata).toStrictEqual(metadata);
resolve();
});
}));
});

0 comments on commit e4700ca

Please sign in to comment.