ProgressNS / nativescript-ui-feedback

This repository is used for customer feedback regarding Telerik UI for NativeScript. The issues system here is used by customers who want to submit their feature requests or vote for existing ones.
Other
115 stars 21 forks source link

IOS - crash on RadListView #1021

Open anuragd7 opened 5 years ago

anuragd7 commented 5 years ago

Please, provide the details below:

Did you verify this is a real problem by searching the NativeScript Forum?

Yes

Tell us about the problem

The itemLoading event on the radlistview triggers a crash.

Which platform(s) does your issue occur on?

IOS

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

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

  1. Access the sample project here
  2. After downloading the project. Run npm i followed by tns run ios
  3. tap on tab4 on the bottom of the screen

the following error log is seen

CONSOLE ERROR [native code]: ERROR TypeError: undefined is not an object (evaluating 'args.view.parent')
CONSOLE ERROR [native code]: ERROR CONTEXT {
"view": {
"def": {
"nodeFlags": 387301445,
"rootNodeFlags": 1,
"nodeMatchedQueries": 6,
"flags": 0,
"nodes": [
{
"nodeIndex": 0,
"parent": null,
"renderParent": null,
"bindingIndex": 0,
"outputIndex": 0,
"checkIndex": 0,
"flags": 1,
"childFlags": 387301445,
"directChildFlags": 1,
"childMatchedQueries": 6,
"matchedQueries": {},
"matchedQueryIds": 0,
"references": {},
"ngContentIndex": null,
"childCount": 41,
"bindings": [],
"bindingFlags": 0,
"outputs": [],
"element": {
"ns": "",
"name": "StackLayout",
"attrs": [
[
"",
"rows",
"auto *"
]
],
"template": null,
"componentProvider": nul<…>
CONSOLE LOG file:///app/profile/profile.component.js:224:20: On item loading called of subjects
CONSOLE LOG file:///app/profile/profile.component.js:224:20: On item loading called of subjects
***** Fatal JavaScript exception - application has been terminated. *****
Native stack trace:
1   0x104ca2c4f NativeScript::reportFatalErrorBeforeShutdown(JSC::ExecState*, JSC::Exception*, bool)
2   0x104cd82c0 NativeScript::FFICallback<NativeScript::ObjCMethodCallback>::ffiClosureCallback(ffi_cif*, void*, void**, void*)
3   0x1055fbd06 ffi_closure_unix64_inner
4   0x1055fc72a ffi_closure_unix64
5   0x108919961 -[TKListViewCell preferredLayoutAttributesFittingAttributes:]
6   0x10a12598d -[UICollectionReusableView _preferredLayoutAttributesFittingAttributes:]
7   0x10a0f0a26 -[UICollectionView _checkForPreferredAttributesInView:originalAttributes:]
8   0x10a0f7880 -[UICollectionView _updateVisibleCellsNow:]
9   0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
10  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
11  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
12  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
13  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
14  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
15  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
16  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
17  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
18  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
19  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
20  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
21  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
22  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
23  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
24  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
25  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
26  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
27  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
28  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
29  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
30  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
31  0x10a0f7fe9 -[UICollectionView _updateVisibleCellsNow:]
JavaScript stack trace:
1   layoutCell@file:///app/tns_modules/nativescript-ui-listview/ui-listview.js:1619:50
2   systemLayoutSizeFittingSize@file:///app/tns_modules/nativescript-ui-listview/ui-listview.js:810:46
3   UIApplicationMain@[native code]
4   start@file:///app/tns_modules/tns-core-modules/application/application.js:275:26
5   run@file:///app/tns_modules/tns-core-modules/application/application.js:303:10
6   bootstrapNativeScriptApp@file:///app/tns_modules/nativescript-angular/platform-common.js:188:26
7   bootstrapApp@file:///app/tns_modules/nativescript-angular/platform-common.js:106:38
8   bootstrapModule@file:///app/tns_modules/nativescript-angular/platform-common.js:90:26
9   anonymous@file:///app/main.js:95:91
10  evaluate@[native code]
11  moduleEvaluation@[native code]
12  promiseReactionJob@[native code]
JavaScript error:
file:///app/tns_modules/nativescript-ui-listview/ui-listview.js:1619:50: JS ERROR RangeError: Maximum call stack size exceeded.
anuragd7 commented 5 years ago

Hi @NickIliev - Let me know if I can help progress this in any way. This is holding up the upgrade to nativescript-ui-listview v 5. The same template worked with version 3.x.

NickIliev commented 5 years ago

@anuragd7 in itemLoading you are calling the onGradeItemLoading method which is is triggering the itemSelected event via args.object.selectItemAt(element). This seems to cause the issue on your side as the itemSelected is in race condition iwht the itemLoading. Try to wrap the call for the selection in minor timeout

setTimeout(() => {
  selectedGradeIndexes.forEach(element => {
    args.object.selectItemAt(element);
  });
}, 300);
anuragd7 commented 5 years ago

@NickIliev. Thanks for the response tried to modify the function by placing the itemSelected in another timeout as suggested. However am still getting the same crash.

onGradeItemLoading(args) {
    const self = this;
    self.gradeTimer = setTimeout(() => {
      console.log("On item loading called of grade");
      const selectedGradeIndexes = self.getSelectedGradeIndexes();
      setTimeout(() => {
        selectedGradeIndexes.forEach(element => {
          args.object.selectItemAt(element);
        });
      }, 300);
      clearTimeout(self.gradeTimer);
    }, 100);
  }
meysam-mahmoodi commented 5 years ago

I had the same error, I changed FlexboxLayout to StackLayout in my RadListView and everything is ok now. Hope can help people who have this problem.

          <GridLayout class="watches">
            <RadListView [items]="watches">
              <ListViewGridLayout tkListViewLayout scrollDirection="vertical" itemHeight="260" spanCount="2">
              </ListViewGridLayout>
              <ng-template tkListItemTemplate let-item="item">
                <StackLayout class="watch-item">
                  <!-- I changed this element from FlexboxLayout to StackLayout  -->
                </StackLayout>
              </ng-template>
            </RadListView>
          </GridLayout>
NathanWalker commented 5 years ago

@anuragd7 @VladimirAmiorkov I see this crash as well on latest of everything (core 6.0, ui-listview 6.4.0)... across pretty normal setups. I've found the TKListViewCell to be problematic and unstable in it's setup. I've found that doing the following, compiling on your own and packing seems to fix all the cases for me:

- (UICollectionViewLayoutAttributes *)preferredLayoutAttributesFittingAttributes:(UICollectionViewLayoutAttributes *)layoutAttributes
{
    UICollectionViewLayoutAttributes *preferred = [super preferredLayoutAttributesFittingAttributes:layoutAttributes];

    // NOTE: The following is unstable and causes app crashes
    // if ((fabs(preferred.frame.size.height - layoutAttributes.frame.size.height) >= 0.5 || fabs(preferred.frame.size.width - layoutAttributes.frame.size.width) >= 0.5)) {

    //     UICollectionViewLayoutInvalidationContext *ctx = [self.ownerListView.layout invalidationContextForPreferredLayoutAttributes:preferred
    //                                                                                                          withOriginalAttributes:layoutAttributes];
    //     [self.ownerListView.layout invalidateLayoutWithContext:ctx];
    // }

    return preferred;
}
VladimirAmiorkov commented 5 years ago

Hi @NathanWalker ,

I am aware of this current state and I have been investigating it the last days. We will do our best to have a solution soon.

NathanWalker commented 5 years ago

No rush @VladimirAmiorkov, appreciate you all looking into it. Just wanted others to know a suitable workaround for now 👍

VladimirAmiorkov commented 5 years ago

That workaround unfortunately is inside the iOS custom framework that we create for the nativescript-ui-listview plugin and its source code is not open source so it is not possible to apply it outside of a official fix. But thank you for this update, if you notice anything else please let me know. Btw the code you commented out is there to add supported for ListViewGridLayoutand ListViewStaggeredLayoutspecifically in projects that use VueJS. In that scenario the initialization of template/context is backwards compared to Vanila NS and Angular NS which is causing some major problems but is the way VueJS handles template/context when parsing a View. Anyway we are on it and hope to release a fix for those problems related to orientation change and crashed on IOS soon.

NathanWalker commented 5 years ago

@VladimirAmiorkov can you just add a hook for Vue in that case. Anything added to any code to handle specific cases for a specific framework, just have a property, isVue = true would absolutely be fine. I think in general the addition of code which is intended to solve a specific issue with a specific framework should always be guarded appropriately as often doing so is likely what gives rises to bugs in general. Most simply via a boolean which is documented to set when you're using a particular framework would be acceptable. Would also allow you to more easily dial cases specifically for different frameworks without affecting others.

VladimirAmiorkov commented 5 years ago

Hi @NathanWalker ,

Adding a hook may seam like an easy option but this functionality is a core functionality which is also shared between all 3 different layouts of the RadListView component and having multiple "paths" in the code of the component would lead to unexpected behaviours between the same scenario but only on different framework (Vanila, Angular and Vue) leading to degradation in the quality of your future development of the plugin.

NathanWalker commented 5 years ago

There's been a crash in angular integrations due to this change since it was added on Jan. 9th: https://github.com/NativeScript/nativescript-ui-listview/commit/3f5275b10d8adb16f791c7c9c2aa4c6c69ccb650#diff-faa473571fbfbe571803ba422bffc77a

That means for 5 months there's been a degradation in quality. Not sure I'm following your reasoning but I trust you nonetheless. The commit states it was specifically modified for Vue integrations however was not guarded to help only Vue integrations. Therefore vanilla and angular have been degraded ever since meaning the future of degradation has long since past?

NathanWalker commented 5 years ago

To be clear this condition !self.ownerListView.isLayoutInProgress && was removed from the guard. My main concern is that when such changes are done for specific framework integrations that they should only be done for that specific integration as to not degrade others. This is why I suggest a boolean since it is clear that the framework integrations behave differently so a simple way to manage each would seem reasonable and acceptable no?

VladimirAmiorkov commented 5 years ago

The above change was indeed made when resolving issue for a specific framework and was not handled correctly. But that condition was never added for a specific framework in the first place so removing it was designed to work for all frameworks, unfortunately we were not able to catch that this breaks other frameworks and causes regression in a timely manner due to its being a very complicated scenario in which this causes issues.

We try to not add any "framework-specific" code to the native iOS and Android code of the library used by nativescript-ui-listview because it would lead to unmaintainable code base (iOS and Android should not be concerned about Angular, Vue etc. :) ).

NathanWalker commented 4 years ago

@VladimirAmiorkov curious to followup on this one. I see this is still an issue, any thoughts?