Skip to content

Commit

Permalink
Fallback to regular subscribe if the store doesn't exist in useSelect (
Browse files Browse the repository at this point in the history
  • Loading branch information
kevin940726 authored Dec 7, 2020
1 parent 563d745 commit 6cfe7a7
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 0 deletions.
154 changes: 154 additions & 0 deletions packages/data/src/components/use-select/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,9 @@ describe( 'useSelect', () => {
expect( selectCount2 ).toHaveBeenCalledTimes( 3 );
expect( TestComponent ).toHaveBeenCalledTimes( 3 );
expect( testInstance.findByType( 'div' ).props.data ).toBe( 1 );

// Test if the unsubscribers get called correctly.
renderer.unmount();
} );

it( 'can subscribe to multiple stores at once', () => {
Expand Down Expand Up @@ -565,5 +568,156 @@ describe( 'useSelect', () => {
childCount: 0,
} );
} );

it( 'handles non-existing stores', () => {
registry.registerStore( 'store-1', counterStore );

let renderer;

const TestComponent = jest.fn( () => {
const state = useSelect(
( select ) => ( {
count1: select( 'store-1' ).getCounter(),
blank: select( 'non-existing-store' )?.getCounter(),
} ),
[]
);

return <div data={ state } />;
} );

act( () => {
renderer = TestRenderer.create(
<RegistryProvider value={ registry }>
<TestComponent />
</RegistryProvider>
);
} );

const testInstance = renderer.root;

expect( testInstance.findByType( 'div' ).props.data ).toEqual( {
count1: 0,
blank: undefined,
} );

act( () => {
registry.dispatch( 'store-1' ).increment();
} );

expect( testInstance.findByType( 'div' ).props.data ).toEqual( {
count1: 1,
blank: undefined,
} );

// Test if the unsubscribers get called correctly.
renderer.unmount();
} );

it( 'handles registration of a non-existing store during rendering', () => {
let renderer;

const TestComponent = jest.fn( () => {
const state = useSelect(
( select ) =>
select( 'not-yet-registered-store' )?.getCounter(),
[]
);

return <div data={ state } />;
} );

act( () => {
renderer = TestRenderer.create(
<RegistryProvider value={ registry }>
<TestComponent />
</RegistryProvider>
);
} );

const testInstance = renderer.root;

expect( testInstance.findByType( 'div' ).props.data ).toBe(
undefined
);

act( () => {
registry.registerStore(
'not-yet-registered-store',
counterStore
);
} );

// This is not ideal, but is the way it's working before and we want to prevent breaking changes.
expect( testInstance.findByType( 'div' ).props.data ).toBe(
undefined
);

act( () => {
registry.dispatch( 'not-yet-registered-store' ).increment();
} );

expect( testInstance.findByType( 'div' ).props.data ).toBe( 1 );

// Test if the unsubscribers get called correctly.
renderer.unmount();
} );

it( 'handles registration of a non-existing store of sub-registry during rendering', () => {
let renderer;

const subRegistry = createRegistry( {}, registry );

const TestComponent = jest.fn( () => {
const state = useSelect(
( select ) =>
select(
'not-yet-registered-child-store'
)?.getCounter(),
[]
);

return <div data={ state } />;
} );

act( () => {
renderer = TestRenderer.create(
<RegistryProvider value={ registry }>
<RegistryProvider value={ subRegistry }>
<TestComponent />
</RegistryProvider>
</RegistryProvider>
);
} );

const testInstance = renderer.root;

expect( testInstance.findByType( 'div' ).props.data ).toBe(
undefined
);

act( () => {
registry.registerStore(
'not-yet-registered-child-store',
counterStore
);
} );

// This is not ideal, but is the way it's working before and we want to prevent breaking changes.
expect( testInstance.findByType( 'div' ).props.data ).toBe(
undefined
);

act( () => {
registry
.dispatch( 'not-yet-registered-child-store' )
.increment();
} );

expect( testInstance.findByType( 'div' ).props.data ).toBe( 1 );

// Test if the unsubscribers get called correctly.
renderer.unmount();
} );
} );
} );
8 changes: 8 additions & 0 deletions packages/data/src/registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,14 @@ export function createRegistry( storeConfigs = {}, parent = null ) {
return stores[ storeName ].subscribe( handler );
}

// Trying to access a store that hasn't been registered,
// this is a pattern rarely used but seen in some places.
// We fallback to regular `subscribe` here for backward-compatibility for now.
// See https://github.com/WordPress/gutenberg/pull/27466 for more info.
if ( ! parent ) {
return subscribe( handler );
}

return parent.__experimentalSubscribeStore( storeName, handler );
}

Expand Down

0 comments on commit 6cfe7a7

Please sign in to comment.