-
Notifications
You must be signed in to change notification settings - Fork 187
fix: Update link tag on stylesheet modification #22504
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
Test Results1 271 files +1 1 271 suites +1 1h 19m 1s ⏱️ + 1m 0s Results for commit 1b53372. ± Comparison against base commit 1777b02. This pull request removes 4 and adds 3 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
String path = resourcePath | ||
.replace(staticResourcesPath, ""); |
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.
To be able to compare with the existing link tag, we should cut off the path part that points to resource dir inside build dir.
Tried to test with Gradle project, but ended up receiving
while changing |
…' into hot-reload-for-css-modifications
flow-server/src/main/java/com/vaadin/flow/server/AppShellRegistry.java
Outdated
Show resolved
Hide resolved
} | ||
String buildFolderName = getBuildFolder(); | ||
File buildFolder = new File(projectFolder, buildFolderName); | ||
if (Constants.TARGET.equals(buildFolderName)) { |
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.
This might not match maven as you can define your own build folder.
Perhaps the better check would be to see if build_folder/classes
exist or build_folder/resources
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.
done, thanks!
…difications # Conflicts: # vaadin-dev-server/src/test/java/com/vaadin/base/devserver/PublicResourcesCssLiveUpdaterTest.java
…' into hot-reload-for-css-modifications
I couldn't really test this with Gradle-based project, because HotswapAgent always passes Might be a bug in HotswapAgent, but I have no idea why this so specific to Gradle. |
I changed it to "IntelliJ IDEA" and now it doesn't stop on the breakpoint in VaadinPlugin or Hotswapper at all, and doesn't hot reload the CSS 🤔 |
When I switch to "IntelliJ IDEA", I can see the following logs when I save the CSS file
|
Maybe some other IDEA setting, e.g. compile on save (don't know if such a thing exists :) )? |
Yes, might be it, because even for V24.9 I have the same observation. |
flow-server/src/main/java/com/vaadin/flow/server/PropertyDeploymentConfiguration.java
Show resolved
Hide resolved
flow-server/src/test/java/com/vaadin/flow/hotswap/HotswapperResourcesTest.java
Outdated
Show resolved
Hide resolved
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.
Tested with Gradle and now it seems to work fine.
It happened once that the first modification of the stylesheet was not propagated, but this is probably something related to my IDE or Gradle.
Added some comment for potential improvements
flow-server/src/main/java/com/vaadin/flow/server/AppShellRegistry.java
Outdated
Show resolved
Hide resolved
flow-server/src/main/java/com/vaadin/flow/hotswap/Hotswapper.java
Outdated
Show resolved
Hide resolved
flow-server/src/test/java/com/vaadin/flow/hotswap/HotswapperResourcesTest.java
Outdated
Show resolved
Hide resolved
createdResources, modifiedResources, deletedResources); | ||
} | ||
|
||
if (anyMatches(".*\\.css", createdResources, modifiedResources, |
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.
Just a note for a future improvement: we could refactor a bit of the code to add a onResourceEvent(ResourceEvent event)
on VaadinHotswapper
interface and move the logic in this method into two separate classes (for styles and translations).
…' into hot-reload-for-css-modifications
Re-tested with maven project - works as expected. |
|
Reverts previously added file watcher for stylesheet files as the static resource folder is already watched by hotswap mechanism.
Updates
link
tag once a modification is detected so stylesheet is hot reloaded.Related-to: #22465