fix: refactor handleEvent method on ChildrenDirective

[impxd]

Pull Request

📖 Description

This is a refactor, not a change in behavior. The intention is to maintain the same functionality while preventing a runtime error that occurs when working with Angular and Zone.js.

You can reproduce the error here: https://stackblitz.com/edit/angular-fast-marco-nctc7f?file=src%2Fapp%2Fapp.component.html

On the file: src/polyfills.ts you can toggle between the bugged tree-item component and the right one.

Also please feel free to tell me if this is doable, and that's because I don't have a lot of context for this ChildrenDirective class like you. Right now this fixes our Angular and Zone.js problems but we don't want to mess with the current behavior.

Thank you.

🎫 Issues

• https://github.com/microsoft/fast/issues/6740

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

• [x] I have included a change request file using $ yarn change
• [ ] I have added tests for my changes.
• [x] I have tested my changes.
• [ ] I have updated the project documentation to reflect my changes.
• [x] I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

• [ ] I have added a new component
• [ ] I have modified an existing component
• [ ] I have updated the definition file
• [ ] I have updated the configuration file

⏭ Next Steps

[impxd]

@microsoft-github-policy-service agree company="Thomson Reuters"

[nicholasrice]

Thanks for putting this up @impxd. I did some more digging because the behavior didn't make any sense to me, and it looks like the core of this issue is that zone.js co-ops the MutationObserver constructor function and returns an object wrapping the original Mutation Observer. It does not invoke mutation observer callback with this wrapped instance. A super-simple, no-dependency repro (other than zone.js) is here: https://codesandbox.io/s/distracted-feather-ddptzt?file=/src/index.mjs

I don't know much about zone.js, but this seems like a bug in the zone.js library to me. Do you know if they aware of this, or if this behavior is by design?

[usrrname]

This issue blocks tree item from rendering on Angular 15 and 16, and was previously discovered on optimized builds of Angular 14+ in https://github.com/microsoft/fast/issues/6740. It pretty much blocks any component using the children directive from rendering in NG 15 and 16 now.

I'm wondering if the FAST team prioritizes integration with certain versions of Angular, React, or moreso Blazor? This is helpful for our team to plan how we can mitigate such errors moving forward.

[chrisdholt]

This issue blocks tree item from rendering on Angular 15 and 16, and was previously discovered on optimized builds of Angular 14+ in #6740. It pretty much blocks any component using the children directive from rendering in NG 15 and 16 now.

I'm wondering if the FAST team prioritizes integration with certain versions of Angular, React, or moreso Blazor? This is helpful for our team to plan how we can mitigate such errors moving forward.

Nope - but Nicks comments point to this being Angular specific and due to their implementation, which weighs heavily on this PR. Has this been investigated or brought to the attention of the Angular team?

[usrrname]

We'll let them know and report back. #Angular #NG

[chrisdholt]

We'll let them know and report back. #Angular #NG

Let us know please! Thx!

[janechu]

Closing as it looks like this has gone stale, also I notice that this would cause a change in behavior as the target is now no longer belonging to the newly created MutationObserver but from the previous observer.