-
-
Notifications
You must be signed in to change notification settings - Fork 256
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
fix(vite-plugin-angular): correctly implement HMR of component styles #1542
Conversation
✅ Deploy Preview for analog-app ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for analog-blog ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for analog-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
647ccb9
to
2fdc932
Compare
✅ Deploy Preview for analog-ng-app ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -14,6 +14,7 @@ import { | |||
ResolvedConfig, | |||
Connect, | |||
} from 'vite'; | |||
import { encapsulateStyle } from '@angular/compiler'; |
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.
Do you know what version this was introduced in? We support Angular v17-19
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.
Looks like it was added in 17.1.0
: angular/angular@f0377d3
I can just make it lazy as we only need it if liveReload is enabled, which will only work in v19+
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.
Good idea
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! 5b3dbfe
(#1542)
fb8845c
to
5b3dbfe
Compare
@mattlewis92 something has broken with the styles in the PR preview https://deploy-preview-1542--analog-app.netlify.app/ |
e682920
to
639e93d
Compare
639e93d
to
9ca000a
Compare
I think this should fix it! |
@mattlewis92 that does fix the build, but the development server is still the same if you run |
I think it was this PR that introduced the regression #1539 |
Yep it's definitely that. The problem seems to be specific to component inline The vite plugin inlines the style into the href of the stylesheet: Whereas the angular cli creates a virtual file that contains the compiled css: |
OK this should do the trick! |
@mattlewis92 thanks! Will you add this back in this PR? #1539 I reverted it on beta so we can keep it all as one change |
Thanks again @mattlewis92. Works great! |
PR Checklist
Follow up to #1539
What is the new behavior?
In #1539 I thought I had fixed HMR, but actually what that change did was disable HMR for component stylesheets and trigger a full reload 🤦♂️
So this followup implements the missing pieces:
handleHotUpdate
plugin hook to reimplement HMR of component stylesViewEncapsulation.Emulated
components by using theencapsulateStyle
helperDoes this PR introduce a breaking change?
Relevant bits from the CLI where I based this code from:
Other information
[optional] What gif best describes this PR or how it makes you feel?