ui-collectionview icon indicating copy to clipboard operation
ui-collectionview copied to clipboard

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

Open felixkrautschuk opened this issue 1 year ago • 16 comments

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:

https://user-images.githubusercontent.com/6443021/228847852-7f1e6533-3602-4067-b05b-a6eeb50509bf.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 avatar Mar 30 '23 13:03 felixkrautschuk

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.

felixkrautschuk avatar Mar 30 '23 13:03 felixkrautschuk

@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

farfromrefug avatar Mar 30 '23 14:03 farfromrefug

@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.

felixkrautschuk avatar Mar 30 '23 15:03 felixkrautschuk

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

farfromrefug avatar Mar 30 '23 16:03 farfromrefug

@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.

farfromrefug avatar Mar 30 '23 16:03 farfromrefug

@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 https://github.com/nativescript-community/ui-collectionview/issues/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 avatar Apr 03 '23 15:04 felixkrautschuk

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:

https://user-images.githubusercontent.com/6443021/232085148-10e91876-b7cf-40d5-9d34-1be12edbf3c1.mov

Updated demo app: ns-collectionview.zip

felixkrautschuk avatar Apr 14 '23 15:04 felixkrautschuk

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

farfromrefug avatar Apr 14 '23 15:04 farfromrefug

@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 avatar Apr 14 '23 16:04 felixkrautschuk

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.

felixkrautschuk avatar Apr 17 '23 10:04 felixkrautschuk

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

farfromrefug avatar Apr 17 '23 15:04 farfromrefug

@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.

MrSnoozles avatar Apr 25 '23 17:04 MrSnoozles

@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.

felixkrautschuk avatar Apr 26 '23 14:04 felixkrautschuk

@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.

farfromrefug avatar Apr 26 '23 16:04 farfromrefug

@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

felixkrautschuk avatar May 04 '23 10:05 felixkrautschuk

@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.

darklight365 avatar Jun 21 '23 16:06 darklight365