Skip to content

Commit

Permalink
ObservableMap: optimize unsubscribe and add unit tests (#60892)
Browse files Browse the repository at this point in the history
* ObservableMap: optimize unsubscribe and add unit tests
* Use jest.fn to count function calls
* Inline the getListeners function after TS upgrade to 5.4.5

Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>
Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
  • Loading branch information
4 people authored Apr 19, 2024
1 parent e33cb88 commit ff3afd3
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ export type ObservableMap< K, V > = {
subscribe( name: K, listener: () => void ): () => void;
};

/**
* A key/value map where the individual entries are observable by subscribing to them
* with the `subscribe` methods.
*/
export function observableMap< K, V >(): ObservableMap< K, V > {
const map = new Map< K, V >();
const listeners = new Map< K, Set< () => void > >();
Expand All @@ -24,20 +28,6 @@ export function observableMap< K, V >(): ObservableMap< K, V > {
}
}

function unsubscribe( name: K, listener: () => void ) {
return () => {
const list = listeners.get( name );
if ( ! list ) {
return;
}

list.delete( listener );
if ( list.size === 0 ) {
listeners.delete( name );
}
};
}

return {
get( name ) {
return map.get( name );
Expand All @@ -58,11 +48,22 @@ export function observableMap< K, V >(): ObservableMap< K, V > {
}
list.add( listener );

return unsubscribe( name, listener );
return () => {
list.delete( listener );
if ( list.size === 0 ) {
listeners.delete( name );
}
};
},
};
}

/**
* React hook that lets you observe an individual entry in an `ObservableMap`.
*
* @param map The `ObservableMap` to observe.
* @param name The map key to observe.
*/
export function useObservableValue< K, V >(
map: ObservableMap< K, V >,
name: K
Expand Down
83 changes: 83 additions & 0 deletions packages/components/src/slot-fill/test/observable-map.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/**
* External dependencies
*/
import { render, screen, act } from '@testing-library/react';

/**
* Internal dependencies
*/
import {
observableMap,
useObservableValue,
} from '../bubbles-virtually/observable-map';

describe( 'ObservableMap', () => {
test( 'should observe individual values', () => {
const map = observableMap();

const listenerA = jest.fn();
const listenerB = jest.fn();

const unsubA = map.subscribe( 'a', listenerA );
const unsubB = map.subscribe( 'b', listenerB );

// check that setting `a` doesn't notify the `b` listener
map.set( 'a', 1 );
expect( listenerA ).toHaveBeenCalledTimes( 1 );
expect( listenerB ).toHaveBeenCalledTimes( 0 );

// check that setting `b` doesn't notify the `a` listener
map.set( 'b', 2 );
expect( listenerA ).toHaveBeenCalledTimes( 1 );
expect( listenerB ).toHaveBeenCalledTimes( 1 );

// check that `delete` triggers notifications, too
map.delete( 'a' );
expect( listenerA ).toHaveBeenCalledTimes( 2 );
expect( listenerB ).toHaveBeenCalledTimes( 1 );

// check that the subscription survived the `delete`
map.set( 'a', 2 );
expect( listenerA ).toHaveBeenCalledTimes( 3 );
expect( listenerB ).toHaveBeenCalledTimes( 1 );

// check that unsubscription really works
unsubA();
unsubB();
map.set( 'a', 3 );
expect( listenerA ).toHaveBeenCalledTimes( 3 );
expect( listenerB ).toHaveBeenCalledTimes( 1 );
} );
} );

describe( 'useObservableValue', () => {
test( 'reacts only to the specified key', () => {
const map = observableMap();
map.set( 'a', 1 );

const MapUI = jest.fn( () => {
const value = useObservableValue( map, 'a' );
return <div>value is { value }</div>;
} );

render( <MapUI /> );
expect( screen.getByText( /^value is/ ) ).toHaveTextContent(
'value is 1'
);
expect( MapUI ).toHaveBeenCalledTimes( 1 );

act( () => {
map.set( 'a', 2 );
} );
expect( screen.getByText( /^value is/ ) ).toHaveTextContent(
'value is 2'
);
expect( MapUI ).toHaveBeenCalledTimes( 2 );

// check that setting unobserved map key doesn't trigger a render at all
act( () => {
map.set( 'b', 1 );
} );
expect( MapUI ).toHaveBeenCalledTimes( 2 );
} );
} );

1 comment on commit ff3afd3

@github-actions
Copy link

Choose a reason for hiding this comment

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

Flaky tests detected in ff3afd3.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8754292409
📝 Reported issues:

Please sign in to comment.