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

feat(platform-server): use EventManagerPlugin on the server #24132

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@vikerman
Contributor

vikerman commented May 25, 2018

Previously event handlers on the server were setup directly. This change makes it so that the event registration on the server go through EventManagerPlugin just like on client. This allows us to add custom event registration handlers on the server which allows us to hook up preboot event handlers cleanly.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

The server renderer directly sets up DOM event handlers on the server.

What is the new behavior?

The server renderer uses the EventManagerPlugin system to register DOM event handlers. This is useful to hook up a different event registration on the server - for instance add an attribute for every element that has an event handler so that preboot can query and buffers only those elements.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@googlebot googlebot added the cla: yes label May 25, 2018

@mary-poppins

This comment has been minimized.

mary-poppins commented May 25, 2018

@mary-poppins

This comment has been minimized.

mary-poppins commented May 25, 2018

@mary-poppins

This comment has been minimized.

mary-poppins commented May 25, 2018

@mary-poppins

This comment has been minimized.

mary-poppins commented May 26, 2018

@vikerman vikerman requested review from alxhub and vicb May 29, 2018

@@ -58,6 +59,7 @@ export const SERVER_RENDER_PROVIDERS: Provider[] = [
},
ServerStylesHost,
{provide: SharedStylesHost, useExisting: ServerStylesHost},
{provide: EVENT_MANAGER_PLUGINS, multi: true, useClass: ServerEventManagerPlugin},

This comment has been minimized.

@alxhub

alxhub May 29, 2018

Contributor

Consider using useExisting to noop existing plugins that you don't want to run instead of short-circuiting the whole plugin set.

@alxhub

alxhub approved these changes May 29, 2018

@@ -110,7 +110,7 @@ export class DomEventsPlugin extends EventManagerPlugin {
}
private patchEvent() {
if (!Event || !Event.prototype) {
if (typeof Event === 'undefined' || !Event || !Event.prototype) {

This comment has been minimized.

@vicb

vicb May 29, 2018

Contributor

Please remove || !Event

import {DOCUMENT, ɵgetDOM as getDOM} from '@angular/platform-browser';
@Injectable()
export class ServerEventManagerPlugin {

This comment has been minimized.

@vicb

vicb May 29, 2018

Contributor

Add a comment /* extends EventManagerPlugin which is private*/

@vicb

vicb approved these changes May 29, 2018

@mary-poppins

This comment has been minimized.

mary-poppins commented May 30, 2018

@mary-poppins

This comment has been minimized.

mary-poppins commented May 30, 2018

feat(platform-server): use EventManagerPlugin on the server
Previously event handlers on the server were setup directly. This change makes it so that the event registration on the server go through EventManagerPlugin just like on client. This allows us to add custom event registration handlers on the server which allows us to hook up preboot event handlers cleanly.
@mary-poppins

This comment has been minimized.

mary-poppins commented May 30, 2018

@vicb vicb closed this in d6595eb May 30, 2018

vicb added a commit that referenced this pull request May 30, 2018

feat(platform-server): use EventManagerPlugin on the server (#24132)
Previously event handlers on the server were setup directly. This change makes it so that the event registration on the server go through EventManagerPlugin just like on client. This allows us to add custom event registration handlers on the server which allows us to hook up preboot event handlers cleanly.

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