Skip to content
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

Use same code for HMR with shadowMode #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alancnet
Copy link

What kind of change does this PR introduce?
Prior to this change, addStylesToShadowDOM would ignore styles it had already added. When developing using Hot Module Reload, it is necessary to replace existing style elements instead.

Did you add tests for your changes?
No, there are no existing tests for addStylesShadow.js. Perhaps there should be, but the changes would be larger than the scope of this PR.

If relevant, did you update the README?
No, the README does not reference shadow mode at all. Perhaps it should, but the changes would be larger than the scope of this PR.

Summary

I use HMR in my projects, and templates update as expected. Styles do not. This solves #38

Does this PR introduce a breaking change?

This is not a breaking change.

Other information

@alancnet
Copy link
Author

@sodatea Requesting some visibility here. Thanks!

@gslama-akqa
Copy link

gslama-akqa commented Oct 28, 2019

Hi,
I'm not sure about any potential side effect but for now this fix seems to be working smoothly. It even incidentally solved this issue.
Thank you for the fix @alancnet!

@alancnet
Copy link
Author

@gslama-akqa My pleasure. I have this fix published at https://www.npmjs.com/package/@alancnet/vue-style-loader if you need it.

@jwayne2
Copy link

jwayne2 commented Jan 22, 2020

File lib/addStylesShadow.js was deleted but vue/cli-service/lib/commands/build/resolveWcEntry.js tries to import it. So building production build fails but HMR works correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants