Conversation
- vue3: pass one stable `itemTemplateSelector` function instead of a new arrow function on every render. Each render changed the native property, which on Android cleared the template type map and ran a full `notifyDataSetChanged()`. - android: number template view types by their index in `itemTemplates` instead of request order, so `clearTemplateTypes()` can no longer make an existing view type mean a different template while the RecyclerView keeps reusing the holders created before (cells rendered with the wrong template, e.g. a list item rendered with a header template or an incoming chat message rendered with the outgoing one). - android: when an ObservableArray change cannot be applied (no adapter yet, updates suspended or the RecyclerView is computing its layout), schedule a refresh instead of dropping the change silently. Data pushed before the adapter existed left the list empty until some unrelated refresh. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
|
@copilot check |
Co-authored-by: vallemar <15719383+vallemar@users.noreply.github.com>
Contributor
| })); | ||
|
|
||
| const getSlotName = (item: any, index: number, items: ListItem[]) => props.itemTemplateSelector?.(item, index, items) ?? 'default'; | ||
| // Keep ONE stable selector function. Passing a new arrow function on every render |
Member
There was a problem hiding this comment.
those AI comments are far too long. Keep them only 2/3 lines and ONLY if necessary!
| // data). While updates are suspended, `resumeUpdates(true)` is the caller's job. | ||
| if (!this._dataUpdatesSuspended && !this._pendingRefresh) { | ||
| this._pendingRefresh = true; | ||
| setTimeout(() => { |
Member
There was a problem hiding this comment.
setTimeout is never a good solution. Plus what does this fix? when does this happen? a reproducible example? i wont add timeout maybe fixes if we cant reproduce. And we should instead call refresh in onLayout if it is a refresh while computing layout
| if (key !== undefined) { | ||
| return key; | ||
| } | ||
| // After `clearTemplateTypes()` the reverse map is empty until each key is requested |
Member
There was a problem hiding this comment.
it is not a fix. The question is why the getKeyByValue is called with templateStringTypeNumber empty?
This should not happen
Member
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three related Android issues that show up as cells rendered with the wrong template (or an empty list) when a
CollectionViewuses several templates throughitemTemplateSelector. Found while debugging a chat screen (day separators, incoming and outgoing bubbles, typing row) built withnativescript-vue3 on@nativescript/core9.1.Vue 3 component: new
itemTemplateSelectorfunction on every render. The render function passed a fresh arrow function each time, so every re-render of the component (for example when any bound attribute of the<CollectionView>changes, like:opacity) changed the nativeitemTemplateSelectorproperty. On Android that handler callsclearTemplateTypes()andrefresh()(a fullnotifyDataSetChanged()). The selector is now created once insetup().Android: view types were numbered in request order.
templateKeyToNativeItemassigned the next free number to whatever key was asked first (the branch that numbered templates fromitemTemplateswas dead code: the maps are always initialized in the constructor). AfterclearTemplateTypes()the numbering started again from 0, so the same view type could now mean a different template while the RecyclerView kept reusing the holders created with the old numbering. Result: holders (and the Vue cells rendered inside them) bound to items of another template. Named templates now always get their index initemTemplatesas view type (unknown keys start at 100, as the original comment intended), andgetKeyByValuederives the key the same way when the reverse map is empty.Android:
onSourceCollectionChangeddropped changes silently. When the adapter did not exist yet, updates were suspended, or the RecyclerView was computing its layout, the ObservableArray change was simply ignored. Data pushed into the array before the adapter existed left the list empty until some unrelated refresh happened (which, until fix 1, was the spurious refresh caused by the selector). A deferredrefresh()is now scheduled instead. While updates are suspended nothing changes:resumeUpdates(true)is still the caller's responsibility.How to reproduce (before)
Vue 3 app,
CollectionViewwith anitemTemplateSelectorreturning e.g.header/in/out, items in anObservableArray, and any dynamic attribute bound on the component. Fill the array shortly after the page loads and toggle the attribute: some cells come out rendered with a different template than the one the selector returned for their item (or the list stays empty when the data arrived before the adapter was created).Verification
Tested on an Android emulator (Pixel 8, API 35) with
@nativescript/core9.1.1 andnativescript-vue3.0.3, dumping the native view tree withuiautomatorwhile scrolling and while items were inserted, removed and replaced. Before: cells with the wrong template and an empty list on first load. After: every cell matches its item's template and the first page renders as soon as it is pushed.tscreports no new errors for the two changed files. iOS is not affected by 2 and 3 (it reuses cells by template name), but it benefits from 1 (no needless refresh on every render).🤖 Generated with Claude Code