-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Pass empty names
option to Sheet when using StyleSheetManager with custom target
#3159
Pass empty names
option to Sheet when using StyleSheetManager with custom target
#3159
Conversation
@eramdam changes look ok, can you add a regression test? |
That's what I did by adding |
Yes, adding that one line isn't obvious for what it's testing. |
Fair enough! 😄 Will do! |
And done! 8ceffa4 |
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 good, thanks!
Could you also add a changelog entry?
Done! Wasn't completely sure if there was a specific format to follow so I used something similar to the already existing entries. |
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.
That looks great, thank you!
any chance that this can be released? |
Fixes #2973
When looking at the thread for the above issue, I noticed that after undoing the changes from #2937, the issue was fixed.
It seems obvious that the
names
map change was made for a reason and I didn't want to break SSR so I settled with that solution which looks "fine" 😅But maybe there's a better approach, if so I'd be happy to oblige 🙇♂️
I also updated the test for StyleSheetManager in order to mount
Title
both in the parent document and in the iframes, as to trigger the bug in a test environment.