search

NativeScript / nativescript-angular

Integrating NativeScript with Angular
http://docs.nativescript.org/angular/tutorial/ng-chapter-0
Apache License 2.0
1.22k stars 241 forks source link

ngFor broken in child components with delayed initialization #2361

Open OzymandiasTheGreat opened 3 years ago

OzymandiasTheGreat commented 3 years ago

Environment Provide version numbers for the following components (information can be retrieved by running tns info in your project folder or by inspecting the package.json of the project):

Describe the bug

Using ngFor in child components (embedded via template selector in HTML) when child component is initialized with a delay, such as with ngIf or ngSwitch with a false initial value, is broken. The loop executes the correct number of times, but every item is either null or undefined and other loop variables are wrong too, e.g. index is -1 for every iteration.

Edit: After more testing, it seems you don't even need a child component. If ngFor array is set from a promise, ngFor breaks as described.

To Reproduce

Expected behavior

Sample project

https://github.com/OzymandiasTheGreat/nativescript-ngfor-bug It's the ng-blank template with an additional child component. Just running the example prints what I'm describing.

Additional context

edusperoni commented 3 years ago

Hey!

Actually, the issue isn't on ngFor but on how loaded works in NativeScript. Loaded emits when the native view is loaded, even if it means it's in the middle of ChangeDetection. This works sometimes in other places because during navigation and bootstrap the views aren't created synchronously as they are when you're using a delayed ngFor.

ngFor does container.createEmbeddedView(template, null) (null being the context) and then manually patches the context, but loaded is happening during createEmbeddedView, so it passes an invalid context to the event.

The new angular integration has just been released (https://blog.nativescript.org/nativescript-angular-12/index.html) and we're wondering on how to best handle this case. Here are the options and pros/cons:

  1. make all events async. pros: no more events mid-cd and others cons: events are no longer synchronous and can no longer be easily modified. unloaded will never fire as it fires DURING ngOnDestroy (that removes listeners afterwards)

  2. add some kind of event prefix that will fire events async (ngSafe-loaded)='loaded(data)' pros: more power to developers cons: a bit frustrating to have this issue happen and have to use this as a workaround

  3. treat loaded differently and always fire it async pros: same API and everything cons: loaded may fire after unloaded! If you create and destroy a component in the same task it may happen. This could get hairy quickly. Other events could also be affected but only loaded is patched

  4. Do nothing. Developers can solve this pretty easily by adding a custom directive like safeLoaded that is an @Output with a setTimeout(emit(),0). We can then add this to the documentation.

It seems there isn't an easy way out of this. On one hand, the event is firing correctly when it should, on the other, it's frustrating to use loaded and have it work differently depending on how it's used. I'm open to suggestions on how to best handle this scenario.

Part of me says we should make clear that loaded and unloaded are not "safe" events, and another part says we should make them behave like the web, which might just be impossible for unloaded.