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

Fix/styles in vue sfc not injected to shadow dom #784

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

gogoend
Copy link

@gogoend gogoend commented Oct 16, 2023

Details

This PR aims to fix the issue: The style tag in the nested component in content scripts doesn't take effect when using Vue SFC.

What I've done is:

  1. Add a placeholder string VUE_SFC_STYLE_PLACEHOLDER when process the style-raw pipeline in parcel-transformer-vue
  2. Replace the placeholder string in js files with the content of CSS assets in bundler. ( To make it, I copy the dist files of the default js bundler of Parcel, traverse the CSS assets and get their contents. I'm not sure if there is any better way? )

Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I agree to license this contribution under the MIT LICENSE
  • I checked the current PR for duplication.

Contacts

  • (OPTIONAL) Discord ID:

If your PR is accepted, we will award you with the Contributor role on Discord server.

To join the server, visit: https://www.plasmo.com/s/d

@gogoend gogoend closed this Oct 16, 2023
@gogoend
Copy link
Author

gogoend commented Oct 16, 2023

Sorry, I've clicked a wrong button.

@louisgv
Copy link
Contributor

louisgv commented Oct 16, 2023

@gogoend no worries, re-openning

@louisgv louisgv reopened this Oct 16, 2023
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very interesting - I assume you copied the js file from parcel upstream and pruned their flow type.

Any chance we can use their module directly, and do minor tweak if needed? (i.e via subclassing, or via referencing their exported functions?)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use typescript for this file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this copied from upstream?

Copy link
Contributor

@louisgv louisgv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gogoend

I'm very intrigued by this PR - the ability to package a separate bundle for style and inject it directly into the transformer seems very powerful. It's a tad sad that the parcel upstream source is flow-based, forcing downstream consumer to resolve to a lot of module copy hack instead of import their stuff directly with typing lol..

Tho, we were able to pull some of their file. So maybe we can reduce the amount of tech debt/copied over code by doing something similar?

see: https://github.com/PlasmoHQ/plasmo/blob/main/core/parcel-core/src/index.ts#L6-L33

@gogoend
Copy link
Author

gogoend commented Oct 17, 2023

@gogoend no worries, re-openning

emmmm, in fact, I'm trying to do some explorations, and want to view the diff online, but choose the wrong repo, and open this PR by accident. Maybe I should view the diff in my forked repo .

@louisgv
Copy link
Contributor

louisgv commented Oct 17, 2023

@gogoend no worries, re-openning

emmmm, in fact, I'm trying to do some explorations, and want to view the diff online, but choose the wrong repo, and open this PR by accident. Maybe I should view the diff in my forked repo .

oh lol - feel free to do it however you like, not a bad PR at all. It helps with visibility too

@gogoend gogoend marked this pull request as draft October 17, 2023 00:58
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.

2 participants