[typeahead] Empty collection has single "undefined" element

[macias]

Bug description:

If you pass an empty collection (by definition it does not contain any elements) it will happen that it has single "undefined" elements. The key elements to see this are:

uib-typeahead="foo(state) for state in getLocation()"

getLocation simply returns [] -- i.e. empty collection. And now all it takes is just check what will happen in foo. It will get an "undefined" element.

It is bad also because user has to know that as a result to getting undefined it is required to return "undefined" as well (not null, not empty string, but undefined). Otherwise the ng-model binding won't work.

Link to minimally-working plunker that reproduces the issue:

http://plnkr.co/edit/iokMQLjb2HvxETWdgnlg?p=preview

Version of Angular, UIBS, and Bootstrap

Angular: 1.5.5

UIBS: 2.0.1

Bootstrap: 3.3.6

[macias]

After some digging I guess the issue lies elsewhere. Typeahead instead of fetching data from the given source initially (until user types something) uses model as datasource. This is wrong, because model does not have to be the same type of element the datasource for autocompletion is. For example your datasource could be collection of objects type Person (with first_name, last_name, birtday, etc fields) while model could be just first_name. And besides, model is not a datasource, if user would like to make it datasource for autocompletion he/she would put this way.

Plunker: http://plnkr.co/edit/tvWxxabP12Z8SApttkR9?p=preview

When you run the code you will see in console, that the item that was passed to the mapping function says "coming from model".

[wesleycho]

What is happening is that it is executed at

bootstrap/src/typeahead/typeahead.js

Lines 566 to 569 in 0023d1b

	locals[parserResult.itemName] = modelValue;
	candidateViewValue = parserResult.viewMapper(originalScope, locals);
	locals[parserResult.itemName] = undefined;
	emptyViewValue = parserResult.viewMapper(originalScope, locals);

.

What is your rationale for wanting to change this? My intuition would be that since the $scope.foo function should be stateless, this shouldn't really come up as an issue.

[macias]

Thank you for the reply.

Stateless is not an issue here. The issue here is the source of the data (and as the result type of the data). Please consider such code:

uib-typeahead="foo(state) for state in getLocation()"

It says, that the dropdown should be populated with getLocation as datasource. This datasource is (for example) a collection of elements of type State which holds flag, name, area, etc. So foo expect to get State as its input, correct? So far I guess we agree :-).

If I further guarantee that my datasource does not return null or undefined, it is pretty surprising foo has to deal with undefined, or null, or raw string. And currently it has because there are data pushed to foo coming from other party than autocomplete datasource.

So the rationale is to keep things organized -- since user defines what is shown in dropdown, use all entities used for dropdown only, do not reuse them for other purposes. Model (what is shown in editbox) is completely different thing, i.e. the type of the element of datasource and type of the model can differ (currently user has to write extra code to prevent processing data not coming from autocomplete datasource). Of course user could make match editbox and dropdown 1:1 but it should be up to user.

Or in other words (assuming foo is parserResult.viewMapper) it is not legal to call parserResult.viewMapper over undefined, or model, or any other data. The only legal call is over the elements of autocomplete datasource because only in this case there is a guarantee of having valid type.

[wesleycho]

The reason I hesitate to change this is because I suspect it would introduce a significant amount of work for what appears to be minimal gains. I just don't have the time for those sort of issues unfortunately. While I agree generally, we should ideally not be calling this function unnecessarily (it wasn't implemented great in the past is my opinion, but that is prior to my joining the project), I don't see a huge upside in investing the time on this versus other issues.

If you would like to file a PR making this change as well as the appropriate changes that fixes this behavior, I would welcome it, but otherwise I don't expect that it will be changed any time soon, if ever (due to the active development being undergone in the Angular 2 successor, ng-bootstrap).

[macias]

Thank you for fair perspective, sure thing, if there is focus on ng-bootstrap it does not make sense to split the efforts.