-
Notifications
You must be signed in to change notification settings - Fork 926
Engines are not returned to pool "per request" but rather "per Html.React method invocation" #487
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
Got this to work as expected by disabling engine reuse in the config, but IMO, we should be able to configure pooling AND allow the same engine instance to persist for the entire request... and that should be the default behavior. But again, please let me know if there is an explicit reason why you feel the engine(s) should be recycled per call to |
This behavior changed in #270. There is discussion there - it was possible for engines not to be returned to the pool in some cases, which caused a fatal crash when all the engines are gone, so that change avoids the issue entirely. I suppose we could make this configurable but I’m not sure there’s much value encouraging this aside from your specific use case for sharing engine scope across multiple requests, as a render call now counts on side-effects from previous render calls. |
You keep using the word There is a ton of value in being able to rely on the components/modules having access to a single execution context for the lifetime of On this topic, it would also be very nice if the React components were kept "live" as the response is being constructed (using something like jsdom, or cheerio, perhaps - which is something I've done in the past with a different framework to support SSR), with a final call to the I can sense, though, that this is likely to not be viewed as a priority. I'm hoping that I might have some availability to make a fork and play around with this. I assume you'd be open to a PR that supports this model, as a long as it's configurable and the defaults remain as they are now? |
Heh you are correct, I did typo earlier. To make sure I am understanding your use case - are you suggesting to leverage the global JavaScript scope across multiple render calls as an alternative to passing bunch of props at once in to a single render call? If so, would setting props in ‘ViewBag’ via a custom html helper extension, and then referencing those props in the final render call accomplish what you are after? Since render calls are supposed to be pure, I’m not seeing how relying on previous render calls should be a pattern this library should recommend. Maybe a code sample or specific example would help aid this discussion along? |
(Yes, I saw the mobx example above, is there a simpler use case without a state manager that we can use for this discussion?) |
I think we can use mobx and still keep the example simple enough. I made a gist that illustrates the basics of both the need, and the pattern. https://gist.github.com/ryedin/6efcc8c0883bb2b1baf4d9a2f19ef105 I may have gone slightly overboard on the comments in the gist, but I wanted you to get a full picture as to what we are trying to achieve. I can't imagine this pattern is too far fetched or out of place. Also...
Render calls are certainly "pure" (nothing changes that). That does not mean they cannot also be reactive (respond to changes in inputs as the larger page context is built up). Also, think in terms of how things work on the client. There is a single execution context, which is what enables these types of patterns to work. Why not think in the same terms for your SSR solution? (the "live"ness thing can be achieved with a true server-side DOM implementation, but I would be happy with just a single execution context per request for starters) |
Thanks for the gist, that was very helpful. I can see how registering data ahead of time would be useful. Longer response incoming. |
Ok, so. Here's an approach that allows you to register data with Mobx ahead of any render calls, but doesn't rely on executing Javascript until a component needs to be rendered. Registering data ahead of time from a razor file: @Html.ReactRegisterPreloadViewData("viewName", new { a = 1, b = 2 }) React.NET would keep track of that data in a private dictionary for the scope of that request, waiting until a render call happens. You could call this helper from your existing MVC partials. Before a component SSR happens, a register function that you define is invoked, which passes any registered view data in as an argument. import { viewDataStore } from './stores'
function RegisterPreloadViewData(viewDatas) {
viewDataStore.init(); // remove any existing data from the current store, since JS engines get re-used, and your store could have been re-used
for (const viewData of viewDatas) {
viewDataStore.setViewData(viewData.viewName, viewData.data);
}
} Support for this doesn't exist yet, but should be pretty simple to add, and avoids issues around acquiring a single JS engine from the shared pool for the lifetime of the request. |
This allows UI libraries like styled-components to work without any extra code in this project See also reactjs#487 reactjs#538
This allows UI libraries like styled-components to work without any extra code in this project See also reactjs#487 reactjs#538
This allows UI libraries like styled-components to work without any extra code in this project See also reactjs#487 reactjs#538
This allows UI libraries like styled-components to work without any extra code in this project See also reactjs#487 reactjs#538
* Add callbacks for running custom Javascript during component render This allows UI libraries like styled-components to work without any extra code in this project See also #487 #538 * Add babel integration test * Add demo using styled-components * Create abstraction for chaining pre, during, and post render callbacks * Add dummy comments to fix build (address these later) * Remove obsolete method reference * Fix tests * Cleanup some code around RenderFunctions * Expose string property instead of closed-over func with side effects * Fix regression in TransformRender * Add render functions tests * Update sample This still needs to be better organized.. * Fix filename * Fix build * Update xml comments
Uh oh!
There was an error while loading. Please reload this page.
see: https://github.com/reactjs/React.NET/blob/master/src/React.AspNet/HtmlHelperExtensions.cs#L84
I'm wondering why the engine is being returned to the pool after each call to
Html.React
, when in all of the comments/docs you indicate that it should be "per request" (and I believe a true "per request" model would be much better overall performance).Performance concerns aside, the way this is working right now also prevents using things like reactive mobx stores on the server across calls to
Html.React
within a given request context. We want to be able to disconnect components from the data that drives them and use dependency injection (on the js side) to connect them to the data. This prevents us from having to explicitly push ALL props in at the top level (the so-called page-level component). This works fine for the client-side rendering, as expected, because there is only a single js execution context. We had assumed that there would also be a single js execution context on the server (per request), because that would make a lot of sense. However, we soon realized that our injected data (injected via subsequent calls toHtml.React
in various partials) is never made available, and is thus not actually being utilized for SSR.Are there other reasons that I am not able to see that are preventing making the engine be truly a singleton across the entire request, rather than simply per call to
Html.React
?The text was updated successfully, but these errors were encountered: