bug(MatSelectHarness): getOptions() returns results from another harness

[mischkl]

Reproduction

Use StackBlitz to reproduce your issue: https://stackblitz.com/fork/components-issue

Steps to reproduce:

1. Add two mat-selects with the same selector (e.g. id="test") but different ancestor selectors to a page. Both should be multiple-selects. Add 3 options in the first select and no options in the second.
2. Write code that sets options of second select whenever selection within first select changes, so that the second select always contains only the options that are selected in the first select.
3. Use test harness to click one option in first select, then use following code on the second test harness to check that only the single option is available:

    await select.open();
    const optionLabels = Promise.all((await select.getOptions()).map(option => option.getText()));

Expected Behavior

What behavior were you expecting to see?

• There should only be one option in the second select.

Actual Behavior

What behavior did you actually see?

• The options returned by the second select are the same as in the first - i.e. 3 options - even though when inspecting the component instance, I have verified that the options are correct (just one option).

Environment

• Angular: 9.1.7
• CDK/Material: 9.2.3
• Browser(s): Chrome v80
• Operating System (e.g. Windows, macOS, Ubuntu): macOS 10.15

[crisbeto]

This was fixed in #19112, but I'm guessing that the duplicate IDs are throwing it off. We should be able to fix it on our end, but having multiple elements with the same ID on the page is invalid.

You should be able to fix it either by giving them different IDs, or removing the id binding which will cause Material to auto-generate a unique ID.

[devversion]

I just marked this as P5 (feel free to update @crisbeto). Personally, I'm not sure if it would be worth fixing this on our side since I think it's a valid assumption the harness can make (i.e. that there are not multiple selects with the same explicit id).

[mischkl]

The problem is, we are using the ng-dynamic-forms library and there it is not possible to remove the id binding, and so in our project it is fairly common to have form controls with the same ID. Also since we don't have control over any of the other attributes, and as there is currently no support in the harness for e.g. finding a select by label, ID is what we have to go on. I do realize that this isn't correct HTML, though. But I'm wondering, if I were to use an attribute other than ID for the selector would that really solve the problem?

[mischkl]

Just verified that the same behavior occurs regardless of which selector is used to get the harness, as long as the duplicate ID is present.

[crisbeto]

But I'm wondering, if I were to use an attribute other than ID for the selector would that really solve the problem?

Yes, that will solve the problem. We use the ID to tie a select to its overlay panel. The panel uses a scheme that looks like {{ id of the select }}-panel.

Also since we don't have control over any of the other attributes, and as there is currently no support in the harness for e.g. finding a select by label, ID is what we have to go on.

The harnesses support any valid CSS selector so you could use an attribute or a class to distinguish between the selects.

[mischkl]

The harnesses support any valid CSS selector so you could use an attribute or a class to distinguish between the selects.

With the mat-select there is no "label" attribute - it's nested somewhere deep inside the rendered HTML structure, which makes it unusable for a selector. Seeing as the point of these harnesses is to provide an API to the components that shields from having to know about their internals, I would expect a lot more options to be offered by the SelectHarnessFilters - label, placeholder, etc. But now I suspect I'm going off in the direction of a new feature request. EDIT: done - #19471.

In any case as mentioned, we unfortunately don't have control over the the attributes as we are using ng-dynamic-forms to dynamically generate the components. I suspect this bug could come back to bite anyone using such a library. But at least it's good to know how to work around the bug, by avoiding duplicate IDs.

[devversion]

@mischkl Can you elaborate on what you'd expect from the harness here?

I see that you submitted separate issues for better filtering. That's great. Thanks. I don't think the select harness has a bug in the perspective of this issue, and there is no good way for us to resolve this problem as it surfaces through invalid manually set ids. It seems like this is working as expected and it's a bug in ng-dynamic-forms if it sets the same id multiple times.

[mischkl]

I guess it could be considered a bug of ng-dynamic-forms but basically it's the way it works, it assigns IDs based on dynamic form models, and every model is required to have an id property - which is also the name of the angular form control. The problem comes when either a combination of two dynamic forms are on a page, both of which happen to use the same form control names, or when one uses a dynamic form array, in which case a template for a form group is repeated any number of times. Unfortunately this is how it's always been since the beginning of that library and I'd say for the sake of compatibility it's unlikely to change.

My expectation would be that when I use different selectors to filter for a harness, regardless if both of the MatSelects have the same ID, then I should reliably be able to count on the framework being able to differentiate between the two.

I should also mention that this seems to be a problem specifically with getOptions() - when calling getValueText() there is no problem. So just from a developer's perspective (not knowing about the difference involving the cdk overlay pane), it seems inconsistent that things work in one case but not in the other. In my case it took over an hour of scratching my head in frustration and testing every possible change I could think of before I finally decided this was a "bug" and filed it.

[devversion]

Yeah, I personally would definitely call this a bug in ng-dynamic-forms as it should avoid duplicate ids on the same page. I agree that the behavior might be confusing, as shown in your example of getOptions and getValueText. Though I think it's not worth adding special logic to the harness/select component in order to support W3C invalid non-unique ids.

I guess we could solve this issue by having another set of internal ids for the select panel, though it seems contradictory to add that just for the sake of supporting this invalid duplicate-id pattern. Also it could mean that we break apps that rely on the predictable ${id}-panel class name.

[mmalerba]

I don't think we want to add anything just to handle duplicate id's. It sounds like you should open a bug against ng-dynamic-forms