Skip to content

Commit

Permalink
fix: register event connectors on useEffect (#450)
Browse files Browse the repository at this point in the history
* fix: register event connectors on useEffect

Solves issue in React 18 strict mode

* fix(layers): register connectors on mount

* nit

* chore: comment
  • Loading branch information
prevwong authored Aug 28, 2022
1 parent 8a72d4a commit 418f7a8
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 13 deletions.
6 changes: 2 additions & 4 deletions packages/core/src/editor/useInternalEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,9 @@ export function useInternalEditor<C>(
);

useEffect(() => {
return () => {
if (!connectorsUsage) {
return;
}
connectorsUsage.register();

return () => {
connectorsUsage.cleanup();
};
}, [connectorsUsage]);
Expand Down
2 changes: 2 additions & 0 deletions packages/layers/src/layers/LayerContextProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export const LayerContextProvider: React.FC<Omit<
);

useEffect(() => {
connectorsUsage.register();

return () => {
connectorsUsage.cleanup();
};
Expand Down
2 changes: 1 addition & 1 deletion packages/utils/src/EventHandlers/ConnectorRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class ConnectorRegistry {
return newId;
}

private getConnectorId(element: HTMLElement, connectorName: string) {
getConnectorId(element: HTMLElement, connectorName: string) {
const elementId = this.getElementId(element);
return `${connectorName}--${elementId}`;
}
Expand Down
51 changes: 43 additions & 8 deletions packages/utils/src/EventHandlers/EventHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
CraftDOMEvent,
Connector,
ConnectorsUsage,
RegisteredConnector,
} from './interfaces';
import { isEventBlockedByDescendant } from './isEventBlockedByDescendant';

Expand Down Expand Up @@ -93,22 +94,45 @@ export abstract class EventHandlers<O extends Record<string, any> = {}> {

// Track all active connector ids here
// This is so we can return a cleanup method below so the callee can programmatically cleanup all connectors

const activeConnectorIds: Set<string> = new Set();

let canRegisterConnectors = false;
const deferredConnectorsToRegister: Map<
string,
() => RegisteredConnector
> = new Map();

const connectors = Object.entries(handlers).reduce<
Record<string, Connector>
>(
(accum, [name, handler]) => ({
...accum,
[name]: (el, required, options) => {
const connector = this.registry.register(el, {
required,
name,
options,
connector: handler,
});

activeConnectorIds.add(connector.id);
const registerConnector = () => {
const connector = this.registry.register(el, {
required,
name,
options,
connector: handler,
});

activeConnectorIds.add(connector.id);
return connector;
};

/**
* Only register connectors immediately if register() has been called.
* Otherwise, defer registration until register() is called
*/
if (canRegisterConnectors) {
registerConnector();
} else {
deferredConnectorsToRegister.set(
this.registry.getConnectorId(el, name),
registerConnector
);
}

return el;
},
Expand All @@ -118,7 +142,18 @@ export abstract class EventHandlers<O extends Record<string, any> = {}> {

return {
connectors,
register: () => {
canRegisterConnectors = true;

deferredConnectorsToRegister.forEach((registerConnector) => {
registerConnector();
});

deferredConnectorsToRegister.clear();
},
cleanup: () => {
canRegisterConnectors = false;

activeConnectorIds.forEach((connectorId) =>
this.registry.remove(connectorId)
);
Expand Down
1 change: 1 addition & 0 deletions packages/utils/src/EventHandlers/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export type EventHandlerConnectors<
> = ChainableConnectors<ReturnType<H['handlers']>, E>;

export type ConnectorsUsage<H extends EventHandlers> = {
register: () => void;
cleanup: () => void;
connectors: EventHandlerConnectors<H>;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('DerivedEventHandlers', () => {
derivedHandlers = testDerivedEventHandler.handlers;
derivedInstance = testDerivedEventHandler.instance;
derivedConnectorsUsage = derivedInstance.createConnectorsUsage();
derivedConnectorsUsage.register();
});
describe('attaching the connector', () => {
beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ describe('EventHandlers', () => {
handlers = testEventHandler.handlers;
instance = testEventHandler.instance;
connectorsUsage = instance.createConnectorsUsage();
connectorsUsage.register();
});
describe('connectors', () => {
it('should have core connectors', () => {
Expand Down

0 comments on commit 418f7a8

Please sign in to comment.