Renderer support for removing root components or updating parameters on them #34232
Conversation
ghost
added
the
SteveSandersonMS
changed the title
| // during a batch, but if a scenario emerges we can add support. | ||
| _batchBuilder.ComponentDisposalQueue.Enqueue(componentId); | ||
| _rootComponentsLatestParameters?.Remove(componentId); | ||
| ProcessRenderQueue(); |
There was a problem hiding this comment.
Will this trigger the disposal of all the components in the tree? I remember having to do something more "sophisticated" here (triggering an empty render).
There was a problem hiding this comment.
It does - disposal is recursive. There's a unit test for that too.
There was a problem hiding this comment.
Cool, I'm glad we found a simpler way, the way I did it was a bit convoluted
Not regarding this as a breaking change, because it would never have been supported before to call the renderer from outside its sync context.
SteveSandersonMS
force-pushed
the
stevesa/renderer-removecomponent
branch
from
290cd0c
to
a92a11e
Compare
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
@javiercn Is there anything further you want to check here, or is this good to merge? I'm asking because I have a subsequent PR building on this and it would be handy to target |
| @@ -493,7 +493,7 @@ public async Task CreateBinder_NullableDateTimeOffset() | |||
| var expectedValue = new DateTime(2018, 3, 4, 1, 2, 3); | |||
|
|
|||
| // Act | |||
| await binder.InvokeAsync(new ChangeEventArgs() { Value = expectedValue.ToString(CultureInfo.InvariantCulture), }); | |||
| await binder.InvokeAsync(new ChangeEventArgs() { Value = expectedValue.ToString(CultureInfo.CurrentCulture), }); | |||
There was a problem hiding this comment.
This seems to be unrelated, won't CurrentCulture cause issues with machines on other locales?
There was a problem hiding this comment.
The test was wrong before. Binding works with culture-specific values. The parsing was already culture-specific, so it was incorrect to test a culture-invariant input, and was failing on my machine (using months for days, and vice-versa).
The only reason it was passing in CI before was due to the cultural imperialism of CultureInfo.InvariantCulture being equivalent to US date formatting. It should now work on all machines, each of them doing both formatting and parsing with their own local formatting.
There was a problem hiding this comment.
the cultural imperialism of
CultureInfo.InvariantCulturebeing equivalent to US date formatting
lol, fair enough :).
I only brought it up because we ran into issues in the past in this area.
This is needed as part of #27574
The core
Rendererneeds to have APIs for being told to:It's not necessary to add a new API for "supply new parameters" because the existing
RenderRootComponentAsyncactually does that already. The one thing it didn't support was being called while the system was not yet quiescent, so that's what I've added. It just involves juggling the tasks a little differently.