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

feat(optimizer): add qwikVite configuration option optimizerOptions.inlineStylesUpToBytes #5089

Merged
merged 2 commits into from
Sep 5, 2023

Conversation

jaroel
Copy link
Contributor

@jaroel jaroel commented Sep 2, 2023

Closes #5069

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests / types / typos

Description

The Optimizer inlines styles up to 2e4 bytes. This PR makes that value configurable using:

qwikVite({
  optimizerOptions: {
    inlineStylesUpToBytes: 400,
  },
}),

Feedback needed:

  1. is inlineStylesUpToBytes a good enough name?
  2. is placing it inside optimizerOptions the correct scope?

Use cases and why

    1. Inlining 20k bytes of css might be invalid for certain use cases.

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • Added new tests to cover the fix / functionality

@netlify
Copy link

netlify bot commented Sep 2, 2023

Deploy Preview for qwik-insights canceled.

Name Link
🔨 Latest commit 2082689
🔍 Latest deploy log https://app.netlify.com/sites/qwik-insights/deploys/64f7800f2faaba0008146c2f

@jaroel
Copy link
Contributor Author

jaroel commented Sep 2, 2023

This works. Could use some docs and some tests.

@@ -266,6 +267,11 @@ export function createPlugin(optimizerOptions: OptimizerOptions = {}) {
}
opts.csr = !!updatedOpts.csr;

opts.inlineStylesUpToBytes = optimizerOptions.inlineStylesUpToBytes ?? 0;
Copy link
Contributor Author

@jaroel jaroel Sep 2, 2023

Choose a reason for hiding this comment

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

We should add a default value here.
I feel 20k bytes is a bit on the large side to inline. I don't have a good view on what is the right size to inline, input welcome.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe having the option to inline is worth more than inlining by default. Especially when an external .css file can be hashed and cached forever.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should keep the previous behavior unless it is overwritten...

@zanettin zanettin added COMP: optimizer WAITING FOR: team Waiting for one of the core team members to review and reply labels Sep 3, 2023
@@ -266,6 +267,11 @@ export function createPlugin(optimizerOptions: OptimizerOptions = {}) {
}
opts.csr = !!updatedOpts.csr;

opts.inlineStylesUpToBytes = optimizerOptions.inlineStylesUpToBytes ?? 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should keep the previous behavior unless it is overwritten...

Copy link
Contributor

@mhevery mhevery left a comment

Choose a reason for hiding this comment

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

Added my changes

@mhevery mhevery enabled auto-merge (squash) September 5, 2023 19:23
@mhevery
Copy link
Contributor

mhevery commented Sep 5, 2023

Thank you for your contribution ❤️

@jaroel
Copy link
Contributor Author

jaroel commented Sep 5, 2023

Thanks for the fixup, nice touch with the docstring!

@mhevery mhevery merged commit e827c2f into QwikDev:main Sep 5, 2023
22 checks passed
@jaroel jaroel deleted the 5069-inline-styles-up-to-size branch September 5, 2023 20:09
kodiakhq bot referenced this pull request in ascorbic/unpic-img Sep 17, 2023
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@builder.io/qwik](https://qwik.builder.io/) ([source](https://togithub.com/BuilderIO/qwik)) | [`1.2.10` -> `1.2.11`](https://renovatebot.com/diffs/npm/@builder.io%2fqwik/1.2.10/1.2.11) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@builder.io%2fqwik/1.2.11?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@builder.io%2fqwik/1.2.11?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@builder.io%2fqwik/1.2.10/1.2.11?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@builder.io%2fqwik/1.2.10/1.2.11?slim=true)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>BuilderIO/qwik (@&#8203;builder.io/qwik)</summary>

### [`v1.2.11`](https://togithub.com/BuilderIO/qwik/releases/tag/v1.2.11)

[Compare Source](https://togithub.com/BuilderIO/qwik/compare/v1.2.10...v1.2.11)

##### What's Changed

-   Add useId() docs and demo component by [@&#8203;n8sabes](https://togithub.com/n8sabes) in [https://github.com/BuilderIO/qwik/pull/5037](https://togithub.com/BuilderIO/qwik/pull/5037)
-   add n8sabes to contributors by [@&#8203;gioboa](https://togithub.com/gioboa) in [https://github.com/BuilderIO/qwik/pull/5040](https://togithub.com/BuilderIO/qwik/pull/5040)
-   chore: array index can be typed as number by [@&#8203;wtlin1228](https://togithub.com/wtlin1228) in [https://github.com/BuilderIO/qwik/pull/4748](https://togithub.com/BuilderIO/qwik/pull/4748)
-   Modular Forms: Fix incorrect type in example by [@&#8203;RaeesBhatti](https://togithub.com/RaeesBhatti) in [https://github.com/BuilderIO/qwik/pull/5036](https://togithub.com/BuilderIO/qwik/pull/5036)
-   docs: add docs about server$ middleware by [@&#8203;RaeesBhatti](https://togithub.com/RaeesBhatti) in [https://github.com/BuilderIO/qwik/pull/5033](https://togithub.com/BuilderIO/qwik/pull/5033)
-   feat: add jsx-a eslint rule by [@&#8203;kerbelp](https://togithub.com/kerbelp) in [https://github.com/BuilderIO/qwik/pull/4256](https://togithub.com/BuilderIO/qwik/pull/4256)
-   docs: Adding Example for useOnWindow  by [@&#8203;josemartinez111](https://togithub.com/josemartinez111) in [https://github.com/BuilderIO/qwik/pull/4878](https://togithub.com/BuilderIO/qwik/pull/4878)
-   chore: supabase-auth-helpers-qwik: update devDependencies by [@&#8203;gioboa](https://togithub.com/gioboa) in [https://github.com/BuilderIO/qwik/pull/5041](https://togithub.com/BuilderIO/qwik/pull/5041)
-   fix: action type by [@&#8203;wtlin1228](https://togithub.com/wtlin1228) in [https://github.com/BuilderIO/qwik/pull/5034](https://togithub.com/BuilderIO/qwik/pull/5034)
-   chore: add more debug info to build by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5047](https://togithub.com/BuilderIO/qwik/pull/5047)
-   fix: align `function` and `()=>` behavior for JSX transpilation by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5012](https://togithub.com/BuilderIO/qwik/pull/5012)
-   fix(core): correctly handle serialization of Signal of X and naked X by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5013](https://togithub.com/BuilderIO/qwik/pull/5013)
-   fix(core): pass through decorators by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5052](https://togithub.com/BuilderIO/qwik/pull/5052)
-   fix(qwik-city): correct rest route regression by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5054](https://togithub.com/BuilderIO/qwik/pull/5054)
-   fix: QwikInsights path resolution by [@&#8203;gioboa](https://togithub.com/gioboa) in [https://github.com/BuilderIO/qwik/pull/5051](https://togithub.com/BuilderIO/qwik/pull/5051)
-   docs: add qwikInsights vite plugin by [@&#8203;gioboa](https://togithub.com/gioboa) in [https://github.com/BuilderIO/qwik/pull/5055](https://togithub.com/BuilderIO/qwik/pull/5055)
-   docs: change [@&#8203;builder](https://togithub.com/builder).io/qwik-labs version by [@&#8203;gioboa](https://togithub.com/gioboa) in [https://github.com/BuilderIO/qwik/pull/5057](https://togithub.com/BuilderIO/qwik/pull/5057)
-   docs: add prettier as devDependencies ([#&#8203;5059](https://togithub.com/BuilderIO/qwik/issues/5059)) by [@&#8203;gioboa](https://togithub.com/gioboa) in [https://github.com/BuilderIO/qwik/pull/5061](https://togithub.com/BuilderIO/qwik/pull/5061)
-   chore: fix .nvmrc with the right version by [@&#8203;gioboa](https://togithub.com/gioboa) in [https://github.com/BuilderIO/qwik/pull/5060](https://togithub.com/BuilderIO/qwik/pull/5060)
-   docs: Document Relative URLs in Loaders by [@&#8203;n8sabes](https://togithub.com/n8sabes) in [https://github.com/BuilderIO/qwik/pull/5063](https://togithub.com/BuilderIO/qwik/pull/5063)
-   docs: Cookbook recipe for playing audio and video media on iOS devices by [@&#8203;n8sabes](https://togithub.com/n8sabes) in [https://github.com/BuilderIO/qwik/pull/5066](https://togithub.com/BuilderIO/qwik/pull/5066)
-   Revert "docs: Cookbook recipe for playing audio and video media on iOS devices" by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5073](https://togithub.com/BuilderIO/qwik/pull/5073)
-   fix(qwik-city): do not write into a closed stream by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5074](https://togithub.com/BuilderIO/qwik/pull/5074)
-   fix: loader type by [@&#8203;wtlin1228](https://togithub.com/wtlin1228) in [https://github.com/BuilderIO/qwik/pull/5062](https://togithub.com/BuilderIO/qwik/pull/5062)
-   Add media controller to cookbook by [@&#8203;n8sabes](https://togithub.com/n8sabes) in [https://github.com/BuilderIO/qwik/pull/5078](https://togithub.com/BuilderIO/qwik/pull/5078)
-   Fix the loader document by [@&#8203;n8sabes](https://togithub.com/n8sabes) in [https://github.com/BuilderIO/qwik/pull/5077](https://togithub.com/BuilderIO/qwik/pull/5077)
-   feat: i18n $localize starter by [@&#8203;gioboa](https://togithub.com/gioboa) in [https://github.com/BuilderIO/qwik/pull/5070](https://togithub.com/BuilderIO/qwik/pull/5070)
-   Loader doc consistency by [@&#8203;n8sabes](https://togithub.com/n8sabes) in [https://github.com/BuilderIO/qwik/pull/5082](https://togithub.com/BuilderIO/qwik/pull/5082)
-   Enhance Media Controller Cookbook Recipe and fix fluid layout for smaller screens by [@&#8203;n8sabes](https://togithub.com/n8sabes) in [https://github.com/BuilderIO/qwik/pull/5081](https://togithub.com/BuilderIO/qwik/pull/5081)
-   Remove overlooked console.log() from cookbook recipe by [@&#8203;n8sabes](https://togithub.com/n8sabes) in [https://github.com/BuilderIO/qwik/pull/5083](https://togithub.com/BuilderIO/qwik/pull/5083)
-   Update cookbook to stream 1080p video content by [@&#8203;n8sabes](https://togithub.com/n8sabes) in [https://github.com/BuilderIO/qwik/pull/5084](https://togithub.com/BuilderIO/qwik/pull/5084)
-   Update pages.json by [@&#8203;blove](https://togithub.com/blove) in [https://github.com/BuilderIO/qwik/pull/5085](https://togithub.com/BuilderIO/qwik/pull/5085)
-   fix: library starter don't bundle dependencies by [@&#8203;wmertens](https://togithub.com/wmertens) in [https://github.com/BuilderIO/qwik/pull/2008](https://togithub.com/BuilderIO/qwik/pull/2008)
-   Fixed typo in Docs > Integrations > Prisma by [@&#8203;adcar](https://togithub.com/adcar) in [https://github.com/BuilderIO/qwik/pull/5090](https://togithub.com/BuilderIO/qwik/pull/5090)
-   starter: Update deps and code of supabase auth helpers qwik pkg by [@&#8203;diecodev](https://togithub.com/diecodev) in [https://github.com/BuilderIO/qwik/pull/4745](https://togithub.com/BuilderIO/qwik/pull/4745)
-   docs: Image updates - Qwik City  by [@&#8203;hamatoyogi](https://togithub.com/hamatoyogi) in [https://github.com/BuilderIO/qwik/pull/5098](https://togithub.com/BuilderIO/qwik/pull/5098)
-   docs: update the basic projection tutorial by [@&#8203;wtlin1228](https://togithub.com/wtlin1228) in [https://github.com/BuilderIO/qwik/pull/5093](https://togithub.com/BuilderIO/qwik/pull/5093)
-   docs: update the implicit reactivity tutorial by [@&#8203;wtlin1228](https://togithub.com/wtlin1228) in [https://github.com/BuilderIO/qwik/pull/5092](https://togithub.com/BuilderIO/qwik/pull/5092)
-   docs: remove outdated props store tutorial by [@&#8203;wtlin1228](https://togithub.com/wtlin1228) in [https://github.com/BuilderIO/qwik/pull/5095](https://togithub.com/BuilderIO/qwik/pull/5095)
-   fix: remove missing tutorial ([#&#8203;5105](https://togithub.com/BuilderIO/qwik/issues/5105)) by [@&#8203;gioboa](https://togithub.com/gioboa) in [https://github.com/BuilderIO/qwik/pull/5106](https://togithub.com/BuilderIO/qwik/pull/5106)
-   docs: fix "Endpoints" link on Qwik City -> Routing page by [@&#8203;jakovljevic-mladen](https://togithub.com/jakovljevic-mladen) in [https://github.com/BuilderIO/qwik/pull/5107](https://togithub.com/BuilderIO/qwik/pull/5107)
-   chore: release qwik-auth 0.1.1 by [@&#8203;ulic75](https://togithub.com/ulic75) in [https://github.com/BuilderIO/qwik/pull/5103](https://togithub.com/BuilderIO/qwik/pull/5103)
-   docs: update reactivity's counter example by [@&#8203;wtlin1228](https://togithub.com/wtlin1228) in [https://github.com/BuilderIO/qwik/pull/5091](https://togithub.com/BuilderIO/qwik/pull/5091)
-   chore(eslint-plugin-qwik): change eslint to ^8.45 by [@&#8203;adaliszk](https://togithub.com/adaliszk) in [https://github.com/BuilderIO/qwik/pull/5110](https://togithub.com/BuilderIO/qwik/pull/5110)
-   docs: fix mismatch title for implicit reactivity tutorial by [@&#8203;wtlin1228](https://togithub.com/wtlin1228) in [https://github.com/BuilderIO/qwik/pull/5111](https://togithub.com/BuilderIO/qwik/pull/5111)
-   feat(optimizer): add qwikVite configuration option optimizerOptions.inlineStylesUpToBytes by [@&#8203;jaroel](https://togithub.com/jaroel) in [https://github.com/BuilderIO/qwik/pull/5089](https://togithub.com/BuilderIO/qwik/pull/5089)
-   fix(qwikcity): Fix route-matcher regression [#&#8203;5080](https://togithub.com/BuilderIO/qwik/issues/5080) by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5112](https://togithub.com/BuilderIO/qwik/pull/5112)
-   docs(authjs): use pathname in protected route example by [@&#8203;ulic75](https://togithub.com/ulic75) in [https://github.com/BuilderIO/qwik/pull/5115](https://togithub.com/BuilderIO/qwik/pull/5115)
-   starters: Ensure that VSCode emmet uses JSX syntax for self-closing tags by [@&#8203;colelawrence](https://togithub.com/colelawrence) in [https://github.com/BuilderIO/qwik/pull/5116](https://togithub.com/BuilderIO/qwik/pull/5116)
-   fix(eslint): Support explicit export with loader-location by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5121](https://togithub.com/BuilderIO/qwik/pull/5121)
-   fix v1.2 playground by [@&#8203;wmertens](https://togithub.com/wmertens) in [https://github.com/BuilderIO/qwik/pull/5124](https://togithub.com/BuilderIO/qwik/pull/5124)
-   feat: Add route rewriting by [@&#8203;claudioshiver](https://togithub.com/claudioshiver) in [https://github.com/BuilderIO/qwik/pull/5122](https://togithub.com/BuilderIO/qwik/pull/5122)
-   docs(qwik-city): Add docs for routes rewriting by [@&#8203;claudioshiver](https://togithub.com/claudioshiver) in [https://github.com/BuilderIO/qwik/pull/5127](https://togithub.com/BuilderIO/qwik/pull/5127)
-   bug(vite): populate rollup cache after transform by [@&#8203;wmertens](https://togithub.com/wmertens) in [https://github.com/BuilderIO/qwik/pull/5130](https://togithub.com/BuilderIO/qwik/pull/5130)
-   feat(qrl): add resolved by [@&#8203;wmertens](https://togithub.com/wmertens) in [https://github.com/BuilderIO/qwik/pull/5118](https://togithub.com/BuilderIO/qwik/pull/5118)
-   test(qwik-city): add route-matcher case with two rest params by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5133](https://togithub.com/BuilderIO/qwik/pull/5133)
-   Issue 5065 by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5131](https://togithub.com/BuilderIO/qwik/pull/5131)
-   fix(qwik-city): Correctly SSG routes even with `trailingSlash: false` by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5132](https://togithub.com/BuilderIO/qwik/pull/5132)
-   Issue 5126 by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5135](https://togithub.com/BuilderIO/qwik/pull/5135)
-   chore(insights): upgrade drizzle by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5134](https://togithub.com/BuilderIO/qwik/pull/5134)
-   fix(qwik-city): correct validator failed type by [@&#8203;wtlin1228](https://togithub.com/wtlin1228) in [https://github.com/BuilderIO/qwik/pull/5137](https://togithub.com/BuilderIO/qwik/pull/5137)
-   fix(insight): Correct DB returning too much data error by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5139](https://togithub.com/BuilderIO/qwik/pull/5139)
-   Revert "chore(insights): upgrade drizzle" by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5138](https://togithub.com/BuilderIO/qwik/pull/5138)
-   docs: improve descriptions for aws, azure and gcp integrations by [@&#8203;WilliamEspegren](https://togithub.com/WilliamEspegren) in [https://github.com/BuilderIO/qwik/pull/5144](https://togithub.com/BuilderIO/qwik/pull/5144)
-   feat: provide compatibility with angular by [@&#8203;dmitry-stepanenko](https://togithub.com/dmitry-stepanenko) in [https://github.com/BuilderIO/qwik/pull/5119](https://togithub.com/BuilderIO/qwik/pull/5119)
-   1.2.11 by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5145](https://togithub.com/BuilderIO/qwik/pull/5145)

##### New Contributors

-   [@&#8203;josemartinez111](https://togithub.com/josemartinez111) made their first contribution in [https://github.com/BuilderIO/qwik/pull/4878](https://togithub.com/BuilderIO/qwik/pull/4878)
-   [@&#8203;blove](https://togithub.com/blove) made their first contribution in [https://github.com/BuilderIO/qwik/pull/5085](https://togithub.com/BuilderIO/qwik/pull/5085)
-   [@&#8203;adcar](https://togithub.com/adcar) made their first contribution in [https://github.com/BuilderIO/qwik/pull/5090](https://togithub.com/BuilderIO/qwik/pull/5090)
-   [@&#8203;diecodev](https://togithub.com/diecodev) made their first contribution in [https://github.com/BuilderIO/qwik/pull/4745](https://togithub.com/BuilderIO/qwik/pull/4745)
-   [@&#8203;adaliszk](https://togithub.com/adaliszk) made their first contribution in [https://github.com/BuilderIO/qwik/pull/5110](https://togithub.com/BuilderIO/qwik/pull/5110)
-   [@&#8203;colelawrence](https://togithub.com/colelawrence) made their first contribution in [https://github.com/BuilderIO/qwik/pull/5116](https://togithub.com/BuilderIO/qwik/pull/5116)
-   [@&#8203;claudioshiver](https://togithub.com/claudioshiver) made their first contribution in [https://github.com/BuilderIO/qwik/pull/5122](https://togithub.com/BuilderIO/qwik/pull/5122)
-   [@&#8203;WilliamEspegren](https://togithub.com/WilliamEspegren) made their first contribution in [https://github.com/BuilderIO/qwik/pull/5144](https://togithub.com/BuilderIO/qwik/pull/5144)

**Full Changelog**: QwikDev/qwik@v1.2.10...v1.2.11

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 9pm on sunday" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/ascorbic/unpic-img).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi44My4wIiwidXBkYXRlZEluVmVyIjoiMzYuODMuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMP: optimizer WAITING FOR: team Waiting for one of the core team members to review and reply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimizer inlines large blobs of styles
3 participants