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

Minify Inline CSS #4311

Closed
bjunc opened this issue Nov 11, 2018 · 12 comments
Closed

Minify Inline CSS #4311

bjunc opened this issue Nov 11, 2018 · 12 comments

Comments

@bjunc
Copy link

bjunc commented Nov 11, 2018

Version

v2.1.0

Reproduction link

https://nuxtjs.org

Steps to reproduce

If you add CSS via Nuxt config, that CSS is minified and inlined into the page head. Component CSS/SCSS is inlined after the Nuxt config global CSS, but not minified.

What is expected ?

All inline CSS should be minified (both global via Nuxt config, and component CSS).

What is actually happening?

Only global (Nuxt config) CSS is being minified.

Additional comments?

I really don't think a reproduction link should be required. It should be strongly encouraged, but it's often unnecessarily onerous.
Reporting bugs should be easy and quick.

This bug report is available on Nuxt community (#c8121)
@ghost ghost added the cmty:bug-report label Nov 11, 2018
@manniL
Copy link
Member

manniL commented Nov 11, 2018

@bjunc A reproduction link is always encouraged (if the bug is reproducible in any manner). With CodeSandbox (template link) this is a matter of minutes for the reporter and makes debugging for the maintainers easier, which results in faster bug fixes.

Reporting bugs should be easy and quick, no doubts, but reproducing them should be as well! ☺️

In this bug case, it takes time to create the reproduction because several things are unclear:

  • Which mode is affected/which did you try
  • Production or dev environment
  • Page component or "normal components"
  • ...

@manniL
Copy link
Member

manniL commented Nov 11, 2018

About the actual issue:

CSS is almost minified properly.
There is a vue-loader issue that line-breaks aren't removed properly A fix is already in progress:

vuejs/vue-loader#1395
vuejs/component-compiler-utils#30
vuejs/component-compiler-utils#36

@bjunc
Copy link
Author

bjunc commented Nov 11, 2018

Thanks for the quick response.

I understand the desire to minimize triage / diagnose; which is why I said encouraged. I stand by the statement though, that making reproduction examples / repos mandatory is onerous. In this case, I'm not really sure how it could be reproduced without actually building and deploying somewhere. Does CodeSandbox allow this? If not, what exactly would you have liked me to do? Build and deploy to Heroku? Is that still a matter of minutes?

Per the actual issue, thanks for the links. I'll keep an eye on the pull requests.

@manniL
Copy link
Member

manniL commented Nov 11, 2018

@bjunc

If there is really no way to reproduce the issue easily then it's fine from my POV to input a random URL. In your case, a CSB (even in dev mode, because you can't change the default command there, Feature Request is already submitted) would be fine though, so downloading the example and running it on the local PC is easy for maintainers. Screenshots from the problem or a portion of the non-minified code would be fine as well.

I see your point and I agree that there are exceptions in terms of providing a reproduction URL, but I see more bugs without an URL where the reporter could've easily created a reproduction than the ones where reproduction is too hard/tedious/...

@bjunc
Copy link
Author

bjunc commented Nov 11, 2018

I understand. You guys get bombarded with "bugs" that are really just help requests / user error. Insisting on a reproduction URL probably weeds out a lot of those. In this case, I felt it necessary enough to enter a random URL, but I could see how that could be abused (annoying).

Again, thanks for the quick reply.

@manniL
Copy link
Member

manniL commented Nov 11, 2018

@bjunc You are welcome! Thanks for the report and for addressing the reproduction URL topic ☺️

Feedback is always appreciated 👍

@stale
Copy link

stale bot commented Dec 2, 2018

Thanks for your contribution to Nuxt.js!
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you would like this issue to remain open:

  1. Verify that you can still reproduce the issue in the latest version of nuxt-edge
  2. Comment the steps to reproduce it

Issues that are labeled as 🕐Pending will not be automatically marked as stale.

@manniL
Copy link
Member

manniL commented Dec 11, 2018

Solved (see vuejs/component-compiler-utils#40). Updating deps should work out.

@manniL manniL closed this as completed Dec 11, 2018
@bjunc
Copy link
Author

bjunc commented Dec 28, 2018

@manniL which deps? Currently running Nuxt v2.3.4, which was released in November (before your comment above).

@manniL
Copy link
Member

manniL commented Dec 28, 2018

@bjunc yarn upgrade/npm update should update the vue deps nuxt has automatically.

@bjunc
Copy link
Author

bjunc commented Jan 8, 2019

I can confirm, this is resolved.

@ghost
Copy link

ghost commented Jan 8, 2019

This bug-report has been fixed by @manniL.

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

No branches or pull requests

4 participants