Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ios crash on back navigation when using visibility-binding inside itemtemplate #58

Open
felixkrautschuk opened this issue Mar 30, 2023 · 16 comments

Comments

@felixkrautschuk
Copy link

Make sure to check the demo app(s) for sample usage

Make sure to check the existing issues in this repository

If the demo apps cannot help and there is no issue for your problem, tell us about it

Please, ensure your title is less than 63 characters long and starts with a capital
letter.

When navigating forwards and then backwards between multiple instances of a page using a collectionview, the iOS app crashes, when using a visibility binding inside the itemTemplate.
I only occurs when navigating at least 2 times to the same page.
Hard to explain by words, look at the video:

Bildschirmaufnahme.2023-03-30.um.14.14.42.mov

***** Fatal JavaScript exception - application has been terminated. *****
NativeScript encountered a fatal error: Uncaught Error: Invalid visibility value: undefined. Valid values are: "visible", "hidden", "collapse".
at
[visibility:setNative](file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view/index.ios.js:598:0)
at applyPendingNativeSetters(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/properties/index.js:1117:0)
at initNativeView(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/properties/index.js:1076:0)
at onResumeNativeUpdates(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view-base/index.js:877:22)
at _resumeNativeUpdates(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view-base/index.js:351:0)
at onLoaded(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view-base/index.js:305:0)
at onLoaded(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view/view-common.js:108:0)
at (file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view-base/index.js:392:0)
at callFunctionWithSuper(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view-base/index.js:386:0)
at callLoaded(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view-base/index.js:392:0)
at loadView(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view-base/index.js:556:0)
at (file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view-base/index.js:307:0)
at eachChildView(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/layouts/layout-base-common.js:101:0)
at eachChild(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view/view-common.js:793:0)
at onLoaded(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view-base/index.js:306:0)
at onLoaded(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view/view-common.js:108:0)
at (file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/view-base/index.js:392:0)
at callFunctionWithSuper(file:///app/vendor.js<…>

<!-- itemTemplate -->

<StackLayout>
    <Label text="{{ id }}"/>
    <Label text="{{ title }}"/>
    <Label text="text" visibility="{{ isNew ? 'visible' : 'collapse' }}"/>  <!-- causing the issue -->
    <Button text="delete" tap="deleteItem"/>
</StackLayout>
export function onPageLoaded() {
  loadItems();
}

function loadItems() {
  vm.set("isLoading", true);

  Http.request({
      url: "https://pokeapi.co/api/v2/ability/?limit=10000",
      method: "GET"
  }).then((response) => {
     const json = response.content.toJSON();

     vm.get("items").splice(0, vm.get("items").length);
     vm.get("items").push(...json.results.map((data) => {
         return {
            title: data.name,
            id: data.url,
            isNew: false
         }
     }));

     vm.set("isLoading", false);
   }, (e) => {
      vm.set("isLoading", true);
   });
}

export function onItemTap()  {
  Frame.topmost().navigate("collectionview-page");
}

Not sure if this is really related to collectionview or maybe nativescript/core, but the issue only occurs using collectionview for us so far (not with RLV or default ListView), so I decided to report it here.

Which platform(s) does your issue occur on?

  • iOS
  • iOS 16 (also tested 15, 14, ...)
  • emulator and device

Please, provide the following version numbers that your issue occurs with:

  • CLI: 8.4.0
  • Cross-platform modules: 8.5.0 (also tested 8.4.7)
  • Runtime(s): 8.5.0 (also tested 8.4.1)
  • Plugin(s): collectionview 4.0.73

Please, tell us how to recreate the issue in as much detail as possible.

Describe the steps to reproduce it.

Just follow the steps from the video provided above

Is there any code involved?

ns-collectionview.zip

@felixkrautschuk
Copy link
Author

I just noticed that the isue can be prevented by resetting the whole by re-setting the whole ObservableAeeay instead of clearing and pushing items to it.

Instead

vm.get("items").splice(0);
vm.get("items").push(...newItemsArray);

do

vm.set("items", new ObservableArray(newItemsArray));

But actually I thought it would be a better practice to re-use the existing ObservableARray instead of creating a new one every time I load the list items from our server.

@farfromrefug
Copy link
Member

@felixkrautschuk not sure it is directly related to this plugin. The error happens on N side.
Would need to figure out why and where from visibility is set to undefined. Could be a reset issue on N side

@felixkrautschuk
Copy link
Author

@farfromrefug I am also not sure why this plugin should cause the issue. I just noticed that the crash did not appear when doing the same stuff with ListView or RadListView.

@farfromrefug
Copy link
Member

@felixkrautschuk I missed that part ! Will try to look at it when I can

@farfromrefug
Copy link
Member

@felixkrautschuk by the way doing the slice then push is not optimized as you do 2 operations whuh means 2 collectionview updates. Instead do a splice removing all and adding all new ones.

@felixkrautschuk
Copy link
Author

@farfromrefug I tried to remove the items and add new items in one step with using the splice method, but then I see those layout issues again as described in #57 and the crash is also still happening. Both things only happen in our own app, I was not able yet to make them reproducable in the sample app so far.

However I noticed, that there also seems to be a timing issue.
When I comment the http request part in the sample app and instead load the items like that:

...
vm.set("isLoading", true);

setTimeout(() => {
      vm.get("items").splice(0, vm.get("items").length);
      vm.get("items").push(...initialItems);

      vm.set("isLoading", false);
}, 200);
...

... then the app is crashing.
When I increase the delay to 1000ms, the crash does not occur.

@felixkrautschuk
Copy link
Author

Finally, I managed to find out why the layout issue and the crash still orccur in our own app with splicing and pushing items in one step. It's because item count can change as well while navigating forwards and backwards.

When I change the demo app, that it always pushes a random number of items to the list like this:

...
setTimeout(() => {
      vm.get("items").splice(0, vm.get("items").length, ...initialItems.slice(0, randomNumber(1, 20)));
      vm.set("isLoading", false);
}, 300);
...

... then the layout issue and the crash are reproducable in the demo app as well:

Bildschirmaufnahme.2023-04-14.um.16.15.41.mov

Updated demo app:
ns-collectionview.zip

@farfromrefug
Copy link
Member

@felixkrautschuk and it does not happen with listview and rlv?

@felixkrautschuk
Copy link
Author

@farfromrefug with ListView, everything works as expected (no layout issues, no crashes)
With RadListView, the layout issues are already visible on forwards navigation (with collectionview only when navigating backwards) and the app is also crashing on iOS with a different error:

NativeScript encountered a fatal error: Uncaught Error: attempt to delete item 18 from section 0 which only contains 1 items before the update
at
onSourceCollectionChanged(file: app/webpack:/ns-collectionview/node_modules/nativescript-ui-listview/index.ios.js:1658:0)
at onSourceCollectionChangedInternal(file: app/webpack:/ns-collectionview/node_modules/nativescript-ui-listview/common.js:598:0)
at handler(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/ui/core/weak-event-listener/index.js:31:0)
at _handleEvent(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/data/observable/index.js:306:0)
at notify(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/data/observable/index.js:287:0)
at splice(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/data/observable-array/index.js:187:0)
at (file: app/webpack:/ns-collectionview/app/radlistview-page.ts:74:22)
at invoke(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/timer/index.ios.js:54:0)
at TimerTargetImpl.tick(file: app/webpack:/ns-collectionview/node_modules/@nativescript/core/timer/index.ios.js:18:0)

@felixkrautschuk
Copy link
Author

Btw, the crash for RadListView can be prevented bei calling listview.refresh() in the loaded-event of RadListView.
I tried that for collectionview as well, but that did not help.

@farfromrefug
Copy link
Member

@felixkrautschuk sorry dont have much time to look at they right now

@MrSnoozles
Copy link
Contributor

MrSnoozles commented Apr 25, 2023

@felixkrautschuk I've got the exact same error (only on iOS aswell, Android works fine). I think it could be related to bugs in ObservableArray. Somewhere the length of the ObservableArray gets mixed up. I also removed all the .splice calls, as soon as I manually .push items to an existing ObservableArray the app will eventually crash.
If you have found any workaround I'd be glad if you could share. I'm trying to find a solution too.

@felixkrautschuk
Copy link
Author

@MrSnoozles I only found out that I could prevent the crash and the layout issue by creating a new ObservableArray everytime I want to remove/add items, but actually that should not be necessary.

@farfromrefug
Copy link
Member

@felixkrautschuk i tested taht a bit.
To test it the right way i had to change your demo to call loadItems in onNavigatingTo and not onLoaded.

  • Never use onLoaded for that! it does not have the signification you think and can happen too many times
  • i would even say onNavigatingTo is wrong, it shoudl be onNavigatedTo.
    Now that being said something is really wrong with your example. You share the same ObservableArray for all pages and modify it from multiple pages at the same time. It will clearly have unexpected behaviours
    This has bad consequences : The closing pages receive item changes and try to relayout while the page is disappearing.
    This should never happen as UICollectionView 100% does not like that.
    I should mention that ListView does not use UICollectionView.

So for now it is a wont fix here as the example shows clear issues.

@felixkrautschuk
Copy link
Author

@farfromrefug thanks for your investigations and sorry for late reply.

Regarding the time of loading the items: I actually load everything in navigatingTo event in our apps, as it feels smooth. Sometimes the loading process is really fast, so it feels like the data is already there when the page is shown to the user. When doing that in navigatedTo event, it feels really weird as the user gets a blank page or only a placeholder for half a second and then only the loading process begins.

And yes, I have used that "shared" ObservableArray approach for years now with RadListView and ListView as I never had problems with that. But I got your point and I admit, that it is not a good practice.
When creating a freshly new Observable (and ObservableArray) for every page, the crash does not happen anymore.

Just want to add one thing: there is one weird effect with iOS 16.4 (I updated Xcode 14.3 last week).
Loading the items in navigatingTo event is working for CollectionView on iOS, but for iOS 16.4 it is crashing (even when using loaded event):

Error: Invalid batch updates detected: the number of sections and/or items returned by the data source before and/or after performing the batch updates are inconsistent with the updates.
Data source before updates = { 1 section with item counts: [5] }
Data source after updates = { 1 section with item counts: [5] }
Updates = [
Insert item (0 - 0),
Insert item (0 - 1),
Insert item (0 - 2),
Insert item (0 - 3),
Insert item (0 - 4)
]
Collection view: <UICollectionView: 0x7fcd33811000; frame = (0 97.6667; 393 720.333); clipsToBounds = YES; hidden = YES; autoresizesSubviews = NO; tag = 117; gestureRecognizers = <NSArray: 0x6000039fcb70>; backgroundColor = UIExtendedGrayColorSpace 0 0; layer = <CALayer: 0x600003701220>; contentOffset: {0, 0}; contentSize: {393, 557}; adjustedContentInset: {0, 0, 0, 0}; layout: <UICollectionViewFlowLayout: 0x7fcd3291aa20>; dataSource: <CollectionViewDataSource: 0x60000356dcf0>>

At first I thought that it is a timing issue and I am forced to use navigatedTo event, but then I noticed that I can prevent the crash by using a setTimeout even with 0 delay:

export function onPageNavigatingTo(args: NavigatedData) {
  vm = createViewModel();

  page = args.object as Page;
  page.bindingContext = vm;

  setTimeout(() => {
    loadItems();
  }, 0);
}

Do you an explanation for that? When using RadListView (or ListView), this issue is not occuring.

Latest sample app:
ns-collectionview.zip

@darklight365
Copy link
Contributor

@farfromrefug - This seems to be an iOS 16.4 bug: https://developer.apple.com/forums/thread/728797

It looks like a possible solution to get around this case would be to switch from UICollectionViewDataSource to UICollectionViewDiffableDataSource as the data source being used within the plugin codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants