Skip to content
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

Static content hot reload (CSS only for now) #6097

Merged
merged 10 commits into from
Apr 14, 2022

Conversation

SteveSandersonMS
Copy link
Member

@SteveSandersonMS SteveSandersonMS commented Apr 14, 2022

A MetadataUpdateHandler that receives calls to UpdateContent and, if it's a CSS file, changes the corresponding content in all current and future BlazorWebView browser instances, and retains this change even if the page is reloaded.

This is complicated by two main things:

  1. We have multiple levels of overriding how static content is obtained. This also varies by platform. Since we need the hot-reloaded content to take priority over any other platform-specific content provision logic, I had to hook into each platform-specific *WebViewManager's request handler and make it check for hot-reloaded content before going through the normal chain of file providers.
  2. The tooling works in terms of project names and relative file paths, not in terms of URLs. The mapping from (project,path) to url is also a function of whether or not project is an RCL and if not, what is the content root for each webview. Performing this mapping therefore requires threading through more information about content roots and some level of hardcoding conventions (e.g., for RCLs, we assume the URLs are of the form _content/RCL/path and the content root within the RCL project is always wwwroot).
    • It might be possible to improve this further by knowing about the SWA manifest at runtime, but AFAIK the current implementation will cover current requirements, and I'm trying not to mess with the layering even more. We can revisit this in the future if needed.

To limit the complexity, I had the JS-side logic simply refresh all stylesheets whenever we hear that any of them has changed. Technically we could limit it to only the one that has changed, but this involves passing through a bunch more info and doing more (project,path)/URL conversions in the opposite direction, which introduces further risk of this not working in some situation where things are customized. I did implement that logic (see the commit 5865584 where I undid it) but wasn't happy about the level of sophistication this caused. Again, we could change the decision in the future if needed.

@SteveSandersonMS SteveSandersonMS requested a review from a team as a code owner April 14, 2022 12:08
@SteveSandersonMS SteveSandersonMS added the area-blazor Blazor Hybrid / Desktop, BlazorWebView label Apr 14, 2022
// hot reloaded files as being in both an RCL and the app assembly. This works fine if the
// names don't clash, but means that RCL files can override an app file if the name does clash.
// TODO: Either have the tooling tell us whether this is the main application project, or
// have MAUI somehow retain the information about application assembly name to runtime.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I've put in a request to have an extra parameter to give us this information, so I might be able to remove this workaround soon.

Copy link
Member

@Eilon Eilon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to all make sense, but I've never worked in this area before. The pattern seems sound. And good thing all those Manager types are internal now 😁

@SteveSandersonMS
Copy link
Member Author

Thanks Eilon. I'm going to merge right away before this ends up in some merge conflict.

It's very likely that more changes/fixes will be needed to this code before we ship, since the tooling side of the implementation isn't yet done. We expect to have to take ask-mode changes right up to GA to get these things aligned.

@SteveSandersonMS SteveSandersonMS merged commit 79f620d into main Apr 14, 2022
@SteveSandersonMS SteveSandersonMS deleted the stevesa/static-content-hot-reload branch April 14, 2022 18:26
@SteveSandersonMS
Copy link
Member Author

Other team people, please feel free to still post CR comments if you want - I'm happy to make follow-up changes if needed.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Blazor Hybrid / Desktop, BlazorWebView
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants