-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add support for updating scoped css files #16507
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
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
const styleElement = document.querySelector(`link[href^="${path}"]`) || | ||
document.querySelector(`link[href^="${document.baseURI}${path}"]`); | ||
|
||
// Receive a Clear-site-data header. | ||
await fetch('/_framework/clear-browser-cache'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scoped css files can contain links to other css files. We'll try clearing out the http caches to force the browser to re-download.
@@ -23,6 +23,12 @@ public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next) | |||
{ | |||
return app => | |||
{ | |||
app.Map("/_framework/clear-browser-cache", app1 => app1.Run(context => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this behavior will be sticking around for a while. If so, might help to add a comment on why we do this here (AKA move the comment below into the code).
|
||
private static async Task HandleBrowserRefresh(BrowserRefreshServer browserRefreshServer, FileItem fileItem, CancellationToken cancellationToken) | ||
{ | ||
// We'd like an accurate scoped css path, but this needs a lot of work to wire-up now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does moving the CSS generation to source generators make things any easier?
I'm not super familiar with the EnC/source generators side of the house but wonder if that helps here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would remove the need to run msbuild and make it faster. But we would still need to know what the path of the css file would be in markup. It's a bit tricky because it's relative to the wwwroot of the project being served. For instance, in a hosted app, the file turns out to be MyApp.Client.styles.css
even though the project being run is MyApp.Hosted
. But if the bundle were present in the Shared folder, it's path would be _content/MyApp.Shared/MyApp.Shared.styles.css
. Lots of oddities to work thru, so taking the easy path and updating everything makes it cheap.
Nice! Great to see this making it in time :) |
This change adds supports for updating .razor.css and .cshtml.css files. It operates by shelling out to MSBuild and invokes targets that regens scoped css files. This is currently a lot (9x) slower compared to emitting deltas. We'll eventually be able to move creating this file in to the compiler and that should improve things there.
A second issue is that we're not correctly calculating the path of the project's bundled scoped css files. While this isn't hard, it's just a lot of work that would get thrown away once we move to hosting MSBuild in-process (dotnet/aspnetcore#31217). In the meanwhile, updating a scoped css file causes every local css file to be updated. At least in the templates, this doesn't look very poor and should suffice for an early preview.