fix(core): trigger invariant when user doesn't return anything from getItems

[sarahdayan]

When users don't return anything from getItems, they get an unhelpful runtime error:

"Cannot read property 'execute' of undefined"

Reproduction link: https://codesandbox.io/s/wizardly-rgb-i69uy (try typing, then see error)

This is because the Requester API determines whether the returned value is a requester description by looking for an execute property on them. If there's no return statement in getItems, the value is undefined and the runtime throws.

This is fine in production, but not too helpful during development, especially for non-TypeScript users who won't get a type error in their editor. This PR therefore adds an invariant to provide a more helpful development error.

Tests

The test uses a spy on onInput because at the level where we're testing, there's no way to track the error. If we wanted to avoid it, it would require testing the error at a lower level, in onInput.ts (which currently doesn't have a test suite).

The current test is closer to what the user would do. Both strategies would leak implementation details, but this one being more expressive regarding what behavior triggers what consequence, I think it's better.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 3d6e60b:

Sandbox	Source
@algolia/autocomplete-example-github-repositories-custom-plugin	Configuration
@algolia/autocomplete-example-instantsearch	Configuration
@algolia/autocomplete-example-playground	Configuration
@algolia/autocomplete-example-preview-panel-in-modal	Configuration
@algolia/autocomplete-example-react-renderer	Configuration
@algolia/autocomplete-example-starter-algolia	Configuration
@algolia/autocomplete-example-starter	Configuration
@algolia/autocomplete-example-voice-search	Configuration
@algolia/autocomplete-example-vue	Configuration
determined-colden-flk5u	PR

[francoischalifour]

Is the post-resolve check still as useful and understandable?

autocomplete/packages/autocomplete-core/src/resolve.ts

Lines 173 to 178 in 3e78566

	invariant(
	Array.isArray(items),
	`The \`getItems\` function must return an array of items but returned type ${JSON.stringify(
	typeof items
	)}:\n\n${JSON.stringify(items, null, 2)}`
	);

This PR adds a check in pre-resolve, and that one happens in post-resolve. They seem very similar and the wording makes it a bit confusing. I think the only difference is that in post-resolve, we expect a resolved array.

[francoischalifour]

The existing invariant that we have lives in postResolve. For consistency, we can move that one to the preResolve function.

[sarahdayan]

The pre-resolve invariant checks that you did return something from getItems, so that we avoid the runtime error that I reproduced here. It can be an array, a promise, or a description. The post-resolve invariant checks that all resolved values are arrays. They don't assert the same thing.

If you want to avoid having two invariants, we can change the internals of the resolving code so that even if you didn't return anything, it flows and resolves to undefined, and let the post-resolve invariant handle it. Would this work better for you?

[francoischalifour]

@sarahdayan Yes, it would be fewer code paths and errors to grasp user land.

[francoischalifour]

Since we're at it, perhaps we can also display the sourceId so that the error is more helpful?

Suggested change

	`The \`getItems\` function must return an array of items but returned type ${JSON.stringify(
	`The \`getItems\` function from the source "${source.sourceId}" must return an array of items but returned type ${JSON.stringify(

[sarahdayan]

Good idea 👌Addressed in d3b7f40.

[francoischalifour]

[null] is misleading here since users would expect to see undefined.

See suggestion below.

[francoischalifour]

To avoid getting the confusing [null] invariant, perhaps we can have another invariant right above the other, for the special case of getItems returning undefined:

Suggested change

	invariant(
	(items as Array<typeof items>).every(Boolean),
	`The \`getItems\` function must return an array of items but returned ${JSON.stringify(undefined)}.
	Did you forget to return items?
	See: https://www.algolia.com/doc/ui-libraries/autocomplete/core-concepts/sources/#param-getitems
	);

[sarahdayan]

Yes good call. I've added it after the first invariant, because this one assumes for items to be an array (of possibly undefined values). The first invariant throws if items isn't an array, so as we reach the second one we're covered.

See bdc5b95.

[francoischalifour]

That's great, thanks!

One last note to improve DX a little bit more.