bUnit
bUnit copied to clipboard
bUnit-dev
wip/experiment: new renderer
Pull request description
Work in progress!
This should implement a new way of rendering components that allows us to create DOM tree directly using AngleSharps API, as well as create our own component tree that we can navigate. This PR contains a very naïve port of BrowserRenderer.ts and related types from Blazor. There are definitely optimizations and improvements left on the table.
Open questions:
Current it contains a bunch of experiments I am looking for feedback on:
- [ ] Side effect on DOM on event handler trigger
It is possible to update the DOM when an event is triggered in the attached event handler. However, it would be best if we can figure out a way to get AngleSharp to do it for us, so bUnit doesn't have to reimplement the behavior of the browser.
If it has to be bUnit thing, it should only happen once, even if there are multiple event handlers attached to the same element, e.g. two onkeypress on an input element or one onkeyup and one onkeydown.
- [ ] Detached elements/nodes user handling
When a node/element is removed from the DOM during a re-render, should the node go into a "detached" state, which prevents users from using it? E.g. if a user has a local reference to the node in their test, and then after it has been removed, try to access one of its properties, should we throw an exception?
- [ ] Should nodes be read only for users?
Should we prevent users from modifying the DOM tree? If they do, things might blow up on them, like it would if they modify it in the browser with JavaScript.
Related issue: #46
Closed questions:
- [x] Should we include a
TestRenderer.RenderAsync, which blocks until all async life cycle methods complete?
It is possible to create a RenderAsync method in TestRenderer that blocks until there are no async incomplete tasks in the render queue. Should such a method be included or will it just confuse users. If a user uses this method, they will not be able to test rendered markup/state of a component before an async operation completes, e.g. the "loading data scenario".
In some cases, it might just be simpler for users to call var cut = await RenderAsync(renderFragment) if their test is not focused on the particular async stuff in their life cycle methods, but more on the outcome, e.g. the DOM tree after the full render is complete. If they use RenderAsync, they do not have to do a WaitForXXX call to ensure their DOM is in a predictable state.
There would still need to be a Render(renderFragment) that returns the IRenderedComponent directly, such that users are able to test "partial first renders", e.g. the "loading scenario".
- [x] Support Blazor's event types and async event handlers through AngleSharp event system
AngleSharp's Event, EventTarget.AddEventListener, and EventTarget.Dispatch is not compatible with Blazors custom EventArgs types, but they do correctly implement the bubbling etc., so it makes sense to reuse that, instead of reimplementing bubbling etc., like in v1.
To allow users to await async event handlers the proposal is to have a custom AngleSharp Event type named BunitEvent, that carries the Blazor event args type with it, and can collect a list of Task from async event handlers that it can expose as a Task EventDispatchResult => Task.WhenAll(eventDispatchResults) property.
Then the current event trigger extensions methods from v1, e.g. Click(), can be recreated to dispatch BunitEvents with the correct Blazor event args inside, and can have ClickAsync() counter parts, that are await the EventDispatchResult property.
Closes #48.
Should we include a TestRenderer.RenderAsync, which blocks until all async life cycle methods complete It is possible to create a RenderAsync method in TestRenderer that blocks until there are no async incomplete tasks in the render queue. Should such a method be included or will it just confuse users. If a user uses this method, they will not be able to test rendered markup/state of a component before an async operation completes, e.g. the "loading data scenario".
In short: Yes I am in favor of that. For two reasons:
- Convenience: Often times when you create a component which has async logic in
OnInitializedAsyncyou just care for the outcome not the steps in between. Making the test code more compact. I guess this is the average case. You will have maybe 1 test checking whatOnInitializedAsyncdoes (like smaller substeps which updates a loading bar) but every other case doesn't need that (removing the need forWaitForXXXbefore invoking let's say a button click). - I do tend to say it's normal to have async tests in the ASP.NET Core world.
In short: Yes I am in favor of that. For two reasons: ...
I am also leaning towards yes. This will also seem consistent with async versions of the WaitFor methods, and this would actually not be a breaking change from v1, since Render would still be there.
Pause reviewing a bit for now.
I am current just throwing stuff against the wall to understand the inner workings of the renderer. What I am working on now is a port of the TypeScript based renderer. Its not too far from what I have been doing so far, but there are parts I would like to avoid in it, but basing my efforts on the Mobile Blazor Bindings didn't work out, so I am trying that now. The problem with MBBs approach is that they assume that there can only be a single node or element per render frame, but that's not something we control, since we're supporting components written in for HTML, where that is not the case.
Status update: started porting the browser rendering logic in https://github.dev/dotnet/aspnetcore/blob/main/src/Components/Web.JS/src/Rendering/. Right now I try my best to do a 1-to-1 port, even if the code is not at all idiomatic C# and efficient. Then once, everything is work, ill take a stab at tweaking things, while keeping the code close to the original to make it easy to keep in sync.
@linkdotnet if you are familiar with TypeScript, this could use some a sanity check (if you have the time - lots of code to look through - don't sweat it if you don't have time). The relevant files are all under bunit.web/RenderingPort/. I have simply been sitting with the TS code open in one window next to a window with the C# port and gone through it line by line. I still have parts missing from AngleSharpRenderer and EventTypes, but still managed quite a lot so far.
Ps. all tests in \tests\bunit.web.tests\RenderingPort\AngleSharpRendererTest.cs runs now. Its not a lot, but CanTriggerEvents exercises quite a bit of the code.
Hi @SteveSandersonMS, a quick question related to this.
To save you from reading everything here, this is the TLDR: We're experimenting with porting BrowserRenderer.ts and related types/classes from Blazor, since it looks like building and maintaining the DOM tree rather than regenerating it completely at every render will enable some interesting features for us.
Since the TypeScript code/BrowserRenderer is pretty complex, I want to make sure we get everything ported correctly. I have finished porting all Selenium tests in dotnet/aspnetcore/src/Components/test/E2ETest/Tests/ComponentRenderingTestBase.cs, and would appreciate if you could point out other, if any, potential test (suites) that are relevant to port.
Other input is of course welcome as well.
Not sure if it's of any use to you, but there's a similar-ish thing at we did for Mobile Blazor Bindings, i.e., a .NET implementation of the renderer backend (i.e., what's normally implemented in TypeScript for the browser backend). For example you might recognize the kind of logic at https://github.com/dotnet/MobileBlazorBindings/blob/main/src/Microsoft.MobileBlazorBindings.Core/NativeComponentAdapter.cs#L54
You've probably already got your own version of this logic already but letting you know just in case!
Not sure if it's of any use to you, but there's a similar-ish thing at we did for Mobile Blazor Bindings, i.e., a .NET implementation of the renderer backend (i.e., what's normally implemented in TypeScript for the browser backend). For example you might recognize the kind of logic at https://github.com/dotnet/MobileBlazorBindings/blob/main/src/Microsoft.MobileBlazorBindings.Core/NativeComponentAdapter.cs#L54
You've probably already got your own version of this logic already but letting you know just in case!
@SteveSandersonMS my first attempt was actually to import and reuse the MBB project (core parts). However, it make assumptions and simplifications that wont work in this context, as far as I can tell, such as the render tree producing a legal XML/XAML document and not a HTML5 fragment. E.g. it seems that MBB design only allows for an adapter (component?) to only render one element (https://github.com/dotnet/MobileBlazorBindings/blob/main/src/Microsoft.MobileBlazorBindings.Core/NativeComponentAdapter.cs#L273). Perhaps I am misunderstanding the design and using MBB wrong, but did not manage to get it working with HTML5 fragments.
With the current port of BrowserRenderer.ts and ComponentRenderingTestBase.cs tests, code coverage is good for our code. We are missing coverage of the EditType.permutationListEntry and EditType.permutationListEnd related code.
Side effect on DOM on event handler trigger
This is what I'm waiting for. We all know that Blazor rerenders after handling events (https://github.com/dotnet/aspnetcore/issues/17169#issuecomment-555071478).
Unless you override this behavior with IHandleEvent.
I'm expecting that the new renderer will work correctly for both cases. When you do not use EventUtil.AsNonRenderingEventHandler and when you use it. This would help MudBlazor to test some scenarios better.
This has hung around for long enough. Branch is still there if we want to start this up again.