Skip to content

EventSources for SSE in services now have a defined lifetime.#1284

Open
ArthurPxC wants to merge 2 commits into
devfrom
fix/event-source-lifetime
Open

EventSources for SSE in services now have a defined lifetime.#1284
ArthurPxC wants to merge 2 commits into
devfrom
fix/event-source-lifetime

Conversation

@ArthurPxC

Copy link
Copy Markdown
Contributor

Changes made for "EventSources in Web projects are not closed correctly #1255"

@ArthurPxC ArthurPxC self-assigned this Jun 18, 2026
this.eventSource.onmessage = this.onReceived.bind(this);
}

private clearAll() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please check #1282 for the conflict here and the fix i made there and combine the changes🙂

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this will be replaced by proper effect handling. using ngOnInit and signals is not a good idea. We already prepared this in pair, @ArthurPxC will push it later.

url = input('notifications');
api = input('/api/moryx/notifications/stream');
eventSource: EventSource | undefined;
eventSource: EventSource | null = null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the reason you introduced null here instead of undefined which should be used everywhere else? 🤔

this.disconnectEvents();
}

@HostListener('window:beforeunload')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

use component metadate instead of annotation

Comment on lines +175 to +178
@HostListener('window:beforeunload')
onBeforeUnload() {
this._processHolderStreamService.disconnect();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here

Comment on lines +88 to +89


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Format

this.eventSource.onmessage = this.onReceived.bind(this);
}

private clearAll() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this will be replaced by proper effect handling. using ngOnInit and signals is not a good idea. We already prepared this in pair, @ArthurPxC will push it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants