Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Bug] ResourceDictionary loaded from Source does not allow removing RD entries and inconsistent API-wise #13209

Closed
etvorun opened this issue Dec 21, 2020 · 4 comments · Fixed by #13490
Assignees
Labels
blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. e/5 🕔 5 hotreload Forms bugs that affect Hot Reload t/bug 🐛

Comments

@etvorun
Copy link

etvorun commented Dec 21, 2020

Description

Hot Reload needs API to query and modify ResourceDictionaries loaded from Source yet existing API is mostly inconsistent. Looks like ResourceDictoinary uses helper _mergedInstance field which is not properly assessible for hot reload.

Steps to Reproduce

  1. Open attached project
    XamarinApp9.zip
    .
  2. Set Debug x86.
  3. Build and Run.
  4. Click on "Get count of resources" button.

Actual: text stating that RD.Count and RD.MergedDiciionaries.Count are 0.

Expected: RD.Count should be 2.

  1. Click on "Lookup Color1 resource".

Observe: text stating that Color1 is found and its value displayed.

  1. Click on "Delete Color1 resource".

Actual: text stating that Color1 resource is not found.

Expected: text stating that resource deleted.

  1. Click on "Lookup Color1 resource",

Actual: text stating that Color1 is found and its value displayed.

Expected: text stating that Color1 resource is not found.

@etvorun etvorun added t/bug 🐛 blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. s/unverified New report that has yet to be verified hotreload Forms bugs that affect Hot Reload labels Dec 21, 2020
@samhouts samhouts added this to New in Triage Dec 21, 2020
@hartez hartez removed the s/unverified New report that has yet to be verified label Jan 6, 2021
@hartez hartez added this to To do in Other Ready For Work via automation Jan 6, 2021
@hartez hartez removed this from New in Triage Jan 6, 2021
@hartez hartez added the e/5 🕔 5 label Jan 6, 2021
@Redth Redth added this to To do in vNext+1 (5.0.0) Jan 13, 2021
@rmarinho rmarinho moved this from To do to Blockers in vNext+1 (5.0.0) Jan 15, 2021
Other Ready For Work automation moved this from To do to Done Jan 22, 2021
vNext+1 (5.0.0) automation moved this from Blockers to Done Jan 22, 2021
rmarinho pushed a commit that referenced this issue Jan 22, 2021
treat elements from the "Source" RD like if they were part of the actual
dictionary, for Count, and Remove
@BretJohnson
Copy link

BretJohnson commented Feb 2, 2021

I test with SR2 and this is definitely better, with most issues fixed. However, I see that the merged Root.Resources has two entries (right number) but they both have a null key and null value. See screenshot below. So the delete test fails above (as Color1 can't be found in the resource dictionary).

Also, updating the color in CustomRD.xaml doesn't work via Hot Reload, due to this. That's the user impact. It would be great to get a fix in for SR3 if possible.

image

@BretJohnson BretJohnson reopened this Feb 2, 2021
Other Ready For Work automation moved this from Done to In progress Feb 2, 2021
vNext+1 (5.0.0) automation moved this from Done to To do Feb 2, 2021
@hartez hartez added this to To Do in 5.0.0 SR 3 Feb 2, 2021
@hartez hartez moved this from To Fix to Done in 5.0.0 SR 3 Feb 2, 2021
@hartez hartez removed this from To do in vNext+1 (5.0.0) Feb 2, 2021
@hartez hartez moved this from Done to Issue In Progress in 5.0.0 SR 3 Feb 4, 2021
@StephaneDelcroix
Copy link
Member

@BretJohnson the delete succeed (and Remove() returns true), and you can see that after the Remove() the Count is 1, and LookupResource again fails (proving that this has been removed).

We KNOW there's some discrepancies in the way resources are counted, removed, enumerated, but we can not fix this in the XF 5 timeframe as it would be a breaking change for existing users, but a fix is planned for the next iteration.

In the meantime, we believe you should be able to get whatever you need for HR to work with the current state.

(side node: the [null,null] is a example of mismatch between the enumerator and the count)

@hartez hartez added this to Issue In progress in 5.0.0 SR 4 (Planning) Feb 9, 2021
@Redth
Copy link
Member

Redth commented Feb 9, 2021

Closing this issue based on @StephaneDelcroix findings. Please reopen if you have some more info.

@Redth Redth closed this as completed Feb 9, 2021
Other Ready For Work automation moved this from In progress to Done Feb 9, 2021
5.0.0 SR 3 automation moved this from Issue In Progress to Done Feb 9, 2021
5.0.0 SR 4 (Planning) automation moved this from Issue In progress to Done Feb 9, 2021
@BretJohnson
Copy link

I retested myself here & all looks good. Thanks all.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. e/5 🕔 5 hotreload Forms bugs that affect Hot Reload t/bug 🐛
Projects
Development

Successfully merging a pull request may close this issue.

5 participants