Certain dialogs don't open anymore since 5.0.0

[Tlepel]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request
[ ] Other... Please describe:

Current behavior

We're opening a new dialog from within a dialog using dialogService.open, but the return value is undefined and the modal doesn't open.

Expected behavior

The new modal should open

Minimal reproduction of the problem with instructions

I'll see if I can make a reproduction, might be because both components extend the same abstract class, which is also a component

The bug seems to originate from commit b852ea3d1019f3fc2625cbc92af8fdd001f70a4e, the value for componentOrTemplate.name is the same for both components, while the content of componentOrTemplate is actually different.

What is the motivation / use case for changing the behavior?

Environment

Angular version: 17.3.9


Browser:
- [ ] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX

For Tooling issues:
- Node version: XX  
- Platform:  

Others:

[Tlepel]

This bug is triggered because the bundler assigns 2 separate components (classes) to variables with the same name (e.g. _a2, r, etc.). This causes componentOrTemplate.name to be the same for 2 separate components, so if one of the 2 components is opened as dialog, the other won't open because of the changes in b852ea3

[NetanelBasal]

Hi, do you want to open a PR with a fix?

[Tlepel]

Sure, although I'd probably change it to the implementation of v4 with a nanoid() function that generates a random id. If that's okay then I'll have the PR up in a few minutes

[NetanelBasal]

Why nanoid?

[Tlepel]

https://github.com/ngneat/dialog/blob/35dfce30506e36fce14a86aee5228632944ae973/projects/ngneat/dialog/src/lib/dialog.service.ts#L220-L236

Because that's how the id property was populated in 4.1.1, the last version before 5.0.0.

I might be able to change it to use a different, non-random value (a derivative of the component name perhaps), but in that case it still implicitly blocks opening 2 modals of the same component. That would still be a breaking change when compared to 4.1.1, right?

[NetanelBasal]

We changed it to a name because it's unique

[NetanelBasal]

But I see the issue, we need a way to generate a unique id for the component without passing id explicitly

[Tlepel]

Yeah, I'd propose to change

if (isComponent(componentOrTemplate)) {
  mergedConfig.id ??= componentOrTemplate.name;
}

to

if (isComponent(componentOrTemplate)) {
  mergedConfig.id ??= nanoid();
}

In that case, if an id is provided, it'll work as intended, but if no id is provided a random (probably unique) id is generated.

[netanel-utila]

But it will generate a new id each time. The goal of the change was to prevent reopen a dialog when it's opened without passing an id

[Tlepel]

Hmm, right. I've looked through the data available at runtime, but there's no exposed data that can help in that case. There is an internal Angular property ɵcmp that might contain a few selectors in the selectors property, but not all components have selectors

[netanel-utila]

A dialog component will have a selector. Let's use it

[Tlepel]

A different solution would be to map each function to a unique id in a defaultComponentIds: Map<Type<any>, string>. What do you think about that?

[netanel-utila]

Let's go with reflect component metadata and use the selector