-
Notifications
You must be signed in to change notification settings - Fork 787
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(hydrate): support style modes in hydrate modules #5953
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
christian-bromann
force-pushed
the
cb/mode-support-in-hydrate-module
branch
from
August 27, 2024 17:00
5f300ee
to
80b3e86
Compare
tanner-reits
requested changes
Aug 29, 2024
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 is good to go once we get rid of the of the magic strings (i.e. const modeResolutionChain = [];
and var modeResolutionChain = [];
)
@tanner-reits thanks for the review, applied changes as suggested. |
tanner-reits
approved these changes
Aug 30, 2024
Released in Stencil |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the current behavior?
If a Stencil component uses Style Modes they currently aren't recognised in the hydrate module, e.g. calling
renderToString
would render the component without any styles. The problem is that Rollup generates a variable scope for the global runtime and the one for the hydrate module which causesmodeResolutionChain
to be defined twice in the bundle. When callingsetMode
, it would add a mode to themodeResolutionChain
variables defined outside of the hydrate scope, leaving the one inside the hydrate module empty and hence not applying the styles to the component.What is the new behavior?
I tweaked the rollup config to essentially remove all
modeResolutionChain
definitions in the bundle and inject on at the beginning of the bundle by uncommenting a statement within theHYDRATE_FACTORY_INTRO
. Furthermore I added amode
option to theSerializeDocumentOptions
interface to allow setting modes per e.g.renderToString
call.Documentation
Does this introduce a breaking change?
Testing
Added an e2e test for this.
Other information
n/a