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

refactor(core): Allow the container and the listenable element to be … #55586

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
6af29b0
refactor(core): Allow the container and the listenable element to be …
iteriani Apr 30, 2024
ba01a42
fixup! refactor(core): Allow the container and the listenable element…
iteriani Apr 30, 2024
b406441
fixup! refactor(core): Allow the container and the listenable element…
iteriani Apr 30, 2024
c2bbd97
fixup! refactor(core): Allow the container and the listenable element…
iteriani Apr 30, 2024
c786d8a
fixup! refactor(core): Allow the container and the listenable element…
iteriani Apr 30, 2024
6596a07
fixup! refactor(core): Allow the container and the listenable element…
iteriani Apr 30, 2024
a17d287
fixup! refactor(core): Allow the container and the listenable element…
iteriani May 1, 2024
ab609cc
fixup! refactor(core): Allow the container and the listenable element…
iteriani May 1, 2024
4f0cece
fixup! refactor(core): Allow the container and the listenable element…
iteriani May 1, 2024
f7a4a88
fixup! refactor(core): Allow the container and the listenable element…
iteriani May 1, 2024
6fc5cf1
fixup! refactor(core): Allow the container and the listenable element…
iteriani May 2, 2024
83cb5f5
fixup! refactor(core): Allow the container and the listenable element…
iteriani May 2, 2024
bd8ff2c
fixup! refactor(core): Allow the container and the listenable element…
iteriani May 2, 2024
db26ee0
fixup! refactor(core): Allow the container and the listenable element…
iteriani May 2, 2024
11e95a0
fixup! refactor(core): Allow the container and the listenable element…
iteriani May 2, 2024
2726fc7
fixup! refactor(core): Allow the container and the listenable element…
iteriani May 2, 2024
7e6d966
fixup! refactor(core): Allow the container and the listenable element…
iteriani May 2, 2024
e8159de
fixup! refactor(core): Allow the container and the listenable element…
iteriani May 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions goldens/public-api/core/primitives/event-dispatch/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
```ts

// @public
export function bootstrapEventContract(field: string, container: Element, appId: string, events: string[], anyWindow?: any): EventContract;
export function bootstrapEventContract(field: string, container: Element, appId: string, events: string[], earlyJsactionTracker?: any): void;
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll probably need to re-generate golden files, this any would be replaced with Tracker type.


// @public
export class Dispatcher {
Expand Down Expand Up @@ -45,7 +45,7 @@ export class EventContract implements UnrenamedEventContract {
// (undocumented)
static MOUSE_SPECIAL_SUPPORT: boolean;
registerDispatcher(dispatcher: Dispatcher_2, restriction: Restriction): void;
replayEarlyEvents(): void;
replayEarlyEvents(earlyJsactionContainer?: EarlyJsactionDataContainer): void;
}

// @public
Expand Down
33 changes: 22 additions & 11 deletions packages/core/primitives/event-dispatch/src/earlyeventcontract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@

import {createEventInfoFromParameters, EventInfo} from './event_info';

declare global {
interface Window {
_ejsa?: EarlyJsactionData;
}
export declare interface EarlyJsactionDataContainer {
_ejsa?: EarlyJsactionData;
}

/**
Expand All @@ -21,11 +19,17 @@ export declare interface EarlyJsactionData {
// List used to keep track of the early JSAction event types.
et: string[];

// List used to keep track of capture event types.
etc: string[];

// List used to keep track of the JSAction events if using earlyeventcontract.
q: EventInfo[];

// Early Jsaction handler
h: (event: Event) => void;

// Container for listening to events
c: HTMLElement;
}

/**
Expand All @@ -34,10 +38,15 @@ export declare interface EarlyJsactionData {
* late-loaded EventContract.
*/
export class EarlyEventContract {
constructor() {
window._ejsa = {
constructor(
private readonly replaySink: EarlyJsactionDataContainer = window as EarlyJsactionDataContainer,
private readonly container = window.document.documentElement,
) {
this.replaySink._ejsa = {
c: container,
q: [],
et: [],
etc: [],
h: (event: Event) => {
const eventInfo = createEventInfoFromParameters(
event.type,
Expand All @@ -46,19 +55,21 @@ export class EarlyEventContract {
window.document.documentElement,
Date.now(),
);
window._ejsa!.q.push(eventInfo);
this.replaySink._ejsa!.q.push(eventInfo);
},
};
}

/**
* Installs a list of event types for window.document.documentElement.
* Installs a list of event types for container .
*/
addEvents(types: string[]) {
addEvents(types: string[], capture?: boolean) {
const replaySink = this.replaySink._ejsa!;
for (let idx = 0; idx < types.length; idx++) {
const eventType = types[idx];
window._ejsa!.et.push(eventType);
window.document.documentElement.addEventListener(eventType, window._ejsa!.h);
const eventTypes = capture ? replaySink.etc : replaySink.et;
eventTypes.push(eventType);
this.container.addEventListener(eventType, replaySink.h, capture);
}
}
}
17 changes: 12 additions & 5 deletions packages/core/primitives/event-dispatch/src/eventcontract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

import * as a11yClickLib from './a11y_click';
import {ActionResolver} from './action_resolver';
import {EarlyJsactionData} from './earlyeventcontract';
import {EarlyJsactionData, EarlyJsactionDataContainer} from './earlyeventcontract';
import * as eventLib from './event';
import {EventContractContainerManager} from './event_contract_container';
import {
Expand Down Expand Up @@ -255,10 +255,12 @@ export class EventContract implements UnrenamedEventContract {
* in the provided event contract. Once all the events are replayed, it cleans
* up the early contract.
*/
replayEarlyEvents() {
replayEarlyEvents(
earlyJsactionContainer: EarlyJsactionDataContainer = window as EarlyJsactionDataContainer,
) {
// Check if the early contract is present and prevent calling this function
// more than once.
const earlyJsactionData: EarlyJsactionData | undefined = window._ejsa;
const earlyJsactionData: EarlyJsactionData | undefined = earlyJsactionContainer._ejsa;
if (!earlyJsactionData) {
return;
}
Expand All @@ -282,9 +284,14 @@ export class EventContract implements UnrenamedEventContract {
const earlyEventHandler: (event: Event) => void = earlyJsactionData.h;
for (let idx = 0; idx < earlyEventTypes.length; idx++) {
const eventType: string = earlyEventTypes[idx];
window.document.documentElement.removeEventListener(eventType, earlyEventHandler);
earlyJsactionData.c.removeEventListener(eventType, earlyEventHandler);
}
const earlyEventTypesCapture: string[] = earlyJsactionData.etc;
for (let idx = 0; idx < earlyEventTypesCapture.length; idx++) {
const eventType: string = earlyEventTypes[idx];
earlyJsactionData.c.removeEventListener(eventType, earlyEventHandler, /* useCapture */ true);
iteriani marked this conversation as resolved.
Show resolved Hide resolved
}
delete window._ejsa;
delete earlyJsactionContainer._ejsa;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a pre-existing code, but I wanted to mention that we generally try not to use delete operations to avoid potential performance penalties related to deopt in hidden classes in V8. We can set the value to undefined instead. Since this is pre-existing, we can do it in a separate PR.

}

/**
Expand Down
47 changes: 38 additions & 9 deletions packages/core/primitives/event-dispatch/src/register_events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,63 @@
* found in the LICENSE file at https://angular.io/license
*/

import {EarlyEventContract, EarlyJsactionDataContainer} from './earlyeventcontract';
import {EventContractContainer} from './event_contract_container';
import {EventContract} from './eventcontract';

type Container = {[key: string]: {[appId: string]: EarlyJsactionDataContainer}};
iteriani marked this conversation as resolved.
Show resolved Hide resolved

/**
* Provides a factory function for bootstrapping an event contract on a
* window object.
* @param field The property on the window that the event contract will be placed on.
* specified object (by default, exposed on the `window`).
* @param field The property on the object that the event contract will be placed on.
* @param container The container that listens to events
* @param appId A given identifier for an application. If there are multiple apps on the page
* then this is how contracts can be initialized for each one.
* @param events An array of event names that should be listened to.
* @param anyWindow The global window object that should receive the event contract.
* @returns The `event` contract. This is both assigned to `anyWindow` and returned for testing.
* @param earlyJsactionTracker The object that should receive the event contract.
*/
export function bootstrapEventContract(
field: string,
container: Element,
appId: string,
events: string[],
anyWindow: any = window,
earlyJsactionTracker: any = window,
iteriani marked this conversation as resolved.
Show resolved Hide resolved
) {
if (!anyWindow[field]) {
anyWindow[field] = {};
if (!earlyJsactionTracker[field]) {
earlyJsactionTracker[field] = {};
}
const eventContract = new EventContract(new EventContractContainer(container));
anyWindow[field][appId] = eventContract;
earlyJsactionTracker[field][appId] = eventContract;
for (const ev of events) {
eventContract.addEvent(ev);
}
return eventContract;
}

/**
* Provides a factory function for bootstrapping an event contract on a
* specified object (by default, exposed on the `window`).
* @param field The property on the object that the event contract will be placed on.
* @param container The container that listens to events
* @param appId A given identifier for an application. If there are multiple apps on the page
* then this is how contracts can be initialized for each one.
* @param eventTypes An array of event names that should be listened to.
* @param captureEventTypes An array of event names that should be listened to with capture.
* @param earlyJsactionTracker The object that should receive the event contract.
*/
export function bootstrapEarlyEventContract(
field: string,
container: HTMLElement,
appId: string,
eventTypes: string[],
captureEventTypes: string[],
earlyJsactionTracker: Container = window as unknown as Container,
) {
if (!earlyJsactionTracker[field]) {
earlyJsactionTracker[field] = {};
}
earlyJsactionTracker[field][appId] = {};
Comment on lines +63 to +64
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do a tree-shakable dev-mode checks in this code? If yes, it might be great to assert whether earlyJsactionTracker[field][appId] already exists and throw an error (otherwise, we just override what we've stored before).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there's no such thing as a dev mode binary i think...

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we do have some dev-mode flags 1P/3P compatible (via copybara transform) in the signals code.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can also do this in a followup PR (let's not block this PR on this change).

const eventContract = new EarlyEventContract(earlyJsactionTracker[field][appId], container);
eventContract.addEvents(eventTypes);
eventContract.addEvents(captureEventTypes, true);
}
58 changes: 55 additions & 3 deletions packages/core/primitives/event-dispatch/test/eventcontract_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@
import * as cache from '../src/cache';
import {fireCustomEvent} from '../src/custom_events';
import {stopPropagation} from '../src/dispatcher';
import {EarlyEventContract, EarlyJsactionData} from '../src/earlyeventcontract';
import {
EarlyEventContract,
EarlyJsactionData,
EarlyJsactionDataContainer,
} from '../src/earlyeventcontract';
import {
EventContractContainer,
EventContractContainerManager,
Expand All @@ -23,6 +27,10 @@ import {Restriction} from '../src/restriction';

import {safeElement, testonlyHtml} from './html';

declare global {
interface Window extends EarlyJsactionDataContainer {}
}

const domContent = `
<div id="container"></div>

Expand Down Expand Up @@ -182,6 +190,11 @@ const domContent = `
</div>
</div>
</div>
<div id="focus-container">
<div id="focus-action-element" jsaction="focus:handleFocus">
<button id="focus-target-element">Focus Button</button>
</div>
</div>
`;

function getRequiredElementById(id: string) {
Expand Down Expand Up @@ -1884,6 +1897,43 @@ describe('EventContract', () => {
expect(eventInfoWrapper.getAction()?.element).toBe(actionElement);
});

it('early capture events are dispatched', () => {
const container = getRequiredElementById('focus-container');
const actionElement = getRequiredElementById('focus-action-element');
const targetElement = getRequiredElementById('focus-target-element');
const replaySink = {_ejsa: undefined};
const removeEventListenerSpy = spyOn(container, 'removeEventListener').and.callThrough();

const earlyEventContract = new EarlyEventContract(replaySink, container);
earlyEventContract.addEvents(['focus'], true);

targetElement.focus();

const earlyJsactionData: EarlyJsactionData | undefined = replaySink._ejsa;
expect(earlyJsactionData).toBeDefined();
expect(earlyJsactionData!.q.length).toBe(1);
expect(earlyJsactionData!.q[0].event.type).toBe('focus');

const dispatcher = jasmine.createSpy<Dispatcher>('dispatcher');
const eventContract = createEventContract({
eventContractContainerManager: new EventContractContainer(container),
eventTypes: ['focus'],
dispatcher,
});

eventContract.replayEarlyEvents(replaySink);

expect(replaySink._ejsa).toBeUndefined();
expect(removeEventListenerSpy).toHaveBeenCalledTimes(1);
expect(dispatcher).toHaveBeenCalledTimes(2);
const eventInfoWrapper = getLastDispatchedEventInfoWrapper(dispatcher);
expect(eventInfoWrapper.getEventType()).toBe('focus');
expect(eventInfoWrapper.getEvent().type).toBe('focus');
expect(eventInfoWrapper.getTargetElement()).toBe(targetElement);
expect(eventInfoWrapper.getAction()?.name).toBe('handleFocus');
expect(eventInfoWrapper.getAction()?.element).toBe(actionElement);
});

it('early events are dispatched when target is cleared', () => {
const container = getRequiredElementById('click-container');
const actionElement = getRequiredElementById('click-action-element');
Expand Down Expand Up @@ -1942,7 +1992,9 @@ describe('EventContract', () => {
relatedTarget: container,
});

const earlyJsactionData: EarlyJsactionData | undefined = window._ejsa;
const earlyJsactionData: EarlyJsactionData | undefined = (
window as EarlyJsactionDataContainer
)._ejsa;
expect(earlyJsactionData).toBeDefined();
expect(earlyJsactionData!.q.length).toBe(1);
expect(earlyJsactionData!.q[0].event).toBe(mouseOutEvent);
Expand All @@ -1956,7 +2008,7 @@ describe('EventContract', () => {

eventContract.replayEarlyEvents();

expect(window._ejsa).toBeUndefined();
expect((window as EarlyJsactionDataContainer)._ejsa).toBeUndefined();
expect(removeEventListenerSpy).toHaveBeenCalledTimes(1);
expect(dispatcher).toHaveBeenCalledTimes(3);
const eventInfoWrapper = getLastDispatchedEventInfoWrapper(dispatcher);
Expand Down
3 changes: 3 additions & 0 deletions packages/core/test/bundling/defer/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,9 @@
{
"name": "init_dom_triggers"
},
{
"name": "init_earlyeventcontract"
},
{
"name": "init_effect"
},
Expand Down