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: also support passed ES Modules from css-loader in addition to CommonJS format #47

Merged
merged 1 commit into from
Mar 3, 2021

Conversation

foxxyz
Copy link

@foxxyz foxxyz commented Aug 16, 2020

What kind of change does this PR introduce?
Fix for new default behavior of css-loader v4

Did you add tests for your changes?
No, due to small change and backwards compatibility. Happy to add if required.

Summary
Due to css-loader using ES Modules as the new default in v4, this module breaks when upgrading css-loader to latest (see #46, #42). This patch addresses that problem.

Does this PR introduce a breaking change?
No, tested to be backwards compatible.

Other information
Not sure what the future plans are for this module and whether it's still relevant when Vue 3 releases (I didn't see vue-style-loader listed as a dependency on vue3 bundles), so feel free to discard this patch if this module is to be phased out.

Fixes #46, Fixes #42

@meowtec
Copy link

meowtec commented Sep 3, 2020

I think it should be moved here:
https://github.com/vuejs/vue-style-loader/blob/master/index.js#L39

@foxxyz
Copy link
Author

foxxyz commented Sep 14, 2020

I think it should be moved here:
https://github.com/vuejs/vue-style-loader/blob/master/index.js#L39

You're right, this is a much cleaner solution that addresses the problem at the root. Thank you @meowtec

Retested and verified.

@foxxyz foxxyz mentioned this pull request Sep 21, 2020
@privatenumber
Copy link

privatenumber commented Sep 21, 2020

@yyx990803

Can we get this in?

Currently, guides/tutorials for ground-up Webpack setup using the latest css-loader are breaking without this, since late July.

Personally wasted a lot of time thinking it was a SSR issue. I imagine many beginners might have been deterred because they couldn't get past this styling bug.

Minimal bug reproduction here.

@kamilic
Copy link

kamilic commented Oct 10, 2020

any good news?

@lezuber
Copy link

lezuber commented Nov 4, 2020

This issue took me on my learning journey today 5+ hours before I found the relation to the css-loader version.. :/

@foxxyz
Copy link
Author

foxxyz commented Dec 3, 2020

@yyx990803 any updates? do you perhaps have other plans for vue-style-loader? thanks 🙏

Copy link

@SergkeiM SergkeiM left a comment

Choose a reason for hiding this comment

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

Copy link

@privatenumber privatenumber left a comment

Choose a reason for hiding this comment

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

Seems anyone can approve PRs without having merging privileges

@foxxyz
Copy link
Author

foxxyz commented Jan 3, 2021

Seems anyone can approve PRs without having merging privileges

Thanks @privatenumber. That's confusing. Guess I'll wait for @yyx990803

@haoqunjiang haoqunjiang merged commit 8b584bd into vuejs:master Mar 3, 2021
@foxxyz
Copy link
Author

foxxyz commented Mar 3, 2021

@sodatea Thank you! 🙏🏽

@nolimitdev
Copy link

releases https://github.com/vuejs/vue-style-loader/releases are not updated for longer time

@SergkeiM
Copy link

SergkeiM commented Mar 4, 2021

@haoqunjiang
Copy link
Member

@nolimitdev Updated. Thanks for the reminder!

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.

css-loader 4.x.x support? Do we still need vue-style-loader ?
8 participants