-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Better ways to manage prerendering+JSInterop interactions #8786
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I need to think of a good example, but would it be possible to have both [1] and [3]? |
I think the main pain point here is when there is logic that needs to run on OnParametersSetAsync that requires performing a JSInterop call or similar and retrieve results in order to procceed. Initializing JS for an element can be done as you said in OnAfterRender if we are willing to live with making the user keep track of it. As an alternative we might be able to subsume it inside ElementRef by having something like ElementRef.Initialize(()=>...) and have that only be run once for the lifetime of the element ref (by virtue of keeping an _isInit flag on elementref) For a component that needs to render its contents based on some js interop result (for example a component that needs to call into local storage) this is a bit trickier. Perhaps, instead of having a service with a flag, we could have a service with a flag+callback? That way you would do something like
Alternatively
Just ideas. The main drawback of the cascading value is the perf of everyone registering for it no matter what. If we follow one of these other patterns, that goes away (you still need to inject the service). |
This is covered by scenario A above, in that during prerendering you simply can't use JSInterop call output, so you need to render something different. After switching to interactive mode, you become free to make your JSInterop call and then render based on the results of that. |
Yep, I'm not disagreeing with you. I'm giving you alternatives to introducing an additional method. Ultimately, what prevents you from calling How do we model this for the most general case where the client gets disconnected from the server? You are going to have the same problem if you check periodically something using js interop. |
Nothing stops you. That’s exactly how you would deal with scenario A. Only difference is it’s not OnAfterRenderAsync, it needs to be a different event representing the transition into interactive mode. About later disconnection, I don’t consider that part of these scenarios. It’s exactly like periodically invoking some remote HTTP API. You have to handle failures if it’s not fire-and-forget. |
Only because you want it to happen once? In the two implementations we care about, OnAfterRenderAsync only gets called after the display has been updated, at which point you have become interactive, hence my point of simply doing: bool _initialized = false;
void OnInit(){
if(JSRuntime.IsAvailable()){
_initialized = true;
CompleteInitialization();
}
void OnAfterRender(){
if(!_initialized){
_initialized = true;
CompleteInitialization();
StateHasChanged();
}
} If you don't think this is good enough then another method is required, but given that we don't want to add an extra method if we can avoid it, i would consider just providing guidance. Having to override two methods is a bit cumbersome, but how common is this case otherwise? |
Finally, I leave the decision up to you. I'm trying to play a bit of devils advocate here, but I know you'll do the most sensible thing. |
That's great, I totally appreciate it. Thanks for adding your thoughts and questioning the design concepts! I'm entirely open to using Were you specifically planning to change this Also, @danroth27 @rynowak - do you have views on whether |
It's going to take me a few minutes to catch up on this thread - which I need to do anyway 👍 Hold tight |
I think the existing behavior of OnAfterRenderAsync is pretty intuitive.
Actually never mind - you would still need some sort of event when component can decided what to do. |
You are right. Given that’s the case it probably makes more sense to make it run after the client has reconnected. If you are only prerendering the two main cases we are trying to enable are not available (hook js to html element, initialize something based on a JS call) If you need to dispose resources the right thing is to implement IDisposable. OnAfterRender should be consistent between client and server so that libraries don’t have to account for the difference. For that reason I think it’s better if it only runs after you have rendered the html in the browser and have populated element refs, etc. |
Is it odd for this event to exist on Component when it typically |
Whichever way we do it, it will apply both server and client side. In the WASM case, the event will simply occur immediately after rendering (possibly only the first render for a given component, depending how we design it). So the developer’s code would work in both environments. |
Yeah, I don't think we should consider seriously providing different implementations of services depending on context. This would be a complexity explosion for us.
Thanks for listing these, I think the second one was a little less clear in my mind and this helps immensely. Do you think that scenario B also includes things like starting a timer or launching HTTP requests? I'm wondering if scenario B is really just about prerendering or about all of these connected/disconnected scenarios in general. In terms of the overall analysis, I think you're pretty much on. I know I suggested the idea of a cascading value, but this feels like a more fundamental concern. I also want to know more about the I feel like we're accumulating more at a pretty rapid rate, but they all feel first class to me except the I would add an idea to these that we add to I'm spitballing a few alternative designs just to have some alternatives considered (and probably rejected). If you want to proceed with option 3, you'd have my 👍 Would it make sense to do something like this: protected override Task OnInitAsync(ComponentContext context)
{
context.RegisterAfterRenderCallback(OnAfterFirstRender, once: true);
context.RegisterAfterRenderCallback(OnAfterFirstInteractiveRender, interactive: true, once: true);
} We initialize you with a context, and you can register callbacks where you get to decide the semantics. I'm hoping/speculating that this has a similar cost to what we'd need to do to support the alternatives. Another alternative, maybe covered by your option 5 is to just make all our lifecycle methods take a struct context that exposes stateful properties from Another option would be to do the same thing, but expose those stateful properties on |
I'm all up for solving this with guidance I think. Saying that you use OnAfterRender for that.
The guidance would be to not render anything on OnInitAsync and do it instead on OnAfterRender with a flag and call StateHasChanged from there. I'm not sure how common this scenario is, so if you need to do it 1 for every 20 components I think it's fine. We can also have an analyzer tell you this or even refactor the code for you. (The transformation is pretty mechanical)
In this case you would fire a task and call StateHasChanged from inside it.
This should always be possible in OnAfterRenderAsync if not, this is an implementation bug. We should have a consistent minimum guaranteed experience for lifecycle methods across platforms and enforce that in our tests. For example:
This also bring in the question. If you are only prerendering (you are creating a static snapshot of the site for some reason). Should we make that information available to you somehow so that you can make a different decision? (Like not ghosting out the component and producing the actual render) |
No I wasn't thinking that, because you're free to do that even before interactivity has started. The distinctive thing about JSInterop is that you can't do it until the client has connected.
I'm not crazy about having a special
I'm certainly open to that, and appreciate the simplicity. It does potentially lead us down a path of having a whole suite of properties like you have on
This design looks OK, but I don't see significant advantages versus having overridable lifecycle methods. Overridable lifecycle methods are easier to code through intellisense prompts and are less allocatey. Overall, the design I feel best about in this discussion is:
|
Oh sorry @javiercn, I posted without realising you had already added a new comment. Apologies if it seems like I was ignoring you. But having read your comment now I basically agree with all you've said there and think my comment aligns with yours.
I don't think we need to do anything. If a developer is doing this, they know it themselves and can already put in their own logic for that. |
Uh oh!
There was an error while loading. Please reload this page.
Background
I've tried to collect all the issues related to this. Here's what appears to be the full set:
Scenarios
From all the descriptions and comments in the above issues, I extract the following scenarios:
Possible solutions
@inject IPrerenderingContext PrerenderingContext
that lets you evaluatePrerenderingContext.IsPrerendering
anywhere in your lifecycle methods or rendering logicIsPrerendering
has changed fromtrue
tofalse
.<PrerenderingStateProvider>
that cascades down a value of typeIPrerenderingState
with a similarbool
flag.OnParametersSetAsync
, and keep track of whether it's the first time you've seenIsPrerendering==false
, and if so do your JSInterop call. It would work but is pretty inconvenient and hard to guess.OnInteractiveAsync
, that runs exactly once when interactive mode first starts.StateHasChanged
. This means the component doesn't have to re-render if you're not trying to have different outputs in the two modes.[SkipDuringPrerendering]
that means you get blank output during prerendering, but we also track the need to render that component once we switch into interactive mode.OnAfterRenderAsync
lifecycle method would now only fire after we switched into interactive mode.HttpContext
) made available to components either as a DI service or as a cascaded value. In turn, this could have anIsPrerendering
flag.Even though [2] does take care of "knowing when to re-render", I'm not attracted to it because of the perf implications. It would mean that every component that experiences scenario A or B has to register/unregister a listener for the cascaded value update, which if it's all the components in a UI library (e.g., Material design, which needs each component wired up to some JSInterop on init) could be a lot of such instances all at once. Moreover, it forces all such components to re-render once they become interactive, even if they don't need to (e.g., because they only have scenario B and not A, e.g., the UI library again). Option [5] also suffers this, but even worse since it would have to notify and force re-rendering even when unrelated contextual information changes.
I think we can basically ignore [4] too, since it's too blunt an instrument - it doesn't let you render different content during prerendering.
We also see that [1] is not sufficient on its own.
The one I think has most promise is [3]. Generally we strongly try to avoid creating new lifecycle methods, but we were expecting to create one new one for
OnAfterFirstRenderAsync
anyway, i.e., #7842. We could combine the needs of #7842 with the JSInterop+prerendering scenarios, and have the rule that it only runs when interactive (including if switching from prerendered to interactive). That is precisely when you want to run your code for scenario B.cc @rynowak @javiercn for further design thoughts.
The text was updated successfully, but these errors were encountered: