Skip to content

fix: remove unnecessary whitespace when handling classes #5648

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

Merged
merged 2 commits into from
Dec 30, 2023

Conversation

jakovljevic-mladen
Copy link
Contributor

@jakovljevic-mladen jakovljevic-mladen commented Dec 29, 2023

Overview

This PR fixes an extra space being added to the element's class attribute. For example, in my app, I usually have an extra space:

image

What is it?

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

Description

This PR has two commits:

  • the first commit adds a lot of tests for serializeClass and serializeClassWithHost functions;
  • the second commit fixes an issue described in the image above.

The first commit will fail test serializeClassWithHost -> should serialize context with empty $scopeIds$ without a fix from a second commit because expected value is 'bar bar-baz baz-bar', but actual value was ' bar bar-baz baz-bar'. It is fixed with the second commit and the expected values matches actual value.

This is minor and an extra space in the string won't harm anything, but it was still an easy fix.

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

Sorry, something went wrong.

Copy link

netlify bot commented Dec 29, 2023

👷 Deploy request for qwik-insights pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit f94584d

@jakovljevic-mladen jakovljevic-mladen force-pushed the fix-serializeClassWithHost branch from 6f5d258 to f94584d Compare December 29, 2023 12:28
@gioboa gioboa added COMP: styling WAITING FOR: team Waiting for one of the core team members to review and reply labels Dec 29, 2023
@gioboa gioboa requested a review from mhevery December 29, 2023 13:02
@jakovljevic-mladen
Copy link
Contributor Author

I can't make my local e2e test run properly. I'm getting errors like:

Cannot find module '/.../qwik/packages/qwik/dist/optimizer.mjs'
imported from /.../qwik/starters/dev-server.ts

I found that CI executes these lines, but they don't work for me (errors with no matches found: builderio-qwik-distribution/*).

What can I do? How can I create/populate /qwik/dist folder?

@gioboa
Copy link
Member

gioboa commented Dec 29, 2023

You can run pnpm build and you will create dist folders

@jakovljevic-mladen
Copy link
Contributor Author

Thanks @gioboa. I managed to run e2e test locally, but it seems that these tests are failing on main branch as well 🙈

@gioboa
Copy link
Member

gioboa commented Dec 29, 2023

I'll try in my local environment and I will let you know

--- UPDATE ---

I re-run all the checks and it ok now

Copy link
Member

@gioboa gioboa left a comment

Choose a reason for hiding this comment

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

Thanks @jakovljevic-mladen it looks great to me.

Copy link
Member

@wmertens wmertens left a comment

Choose a reason for hiding this comment

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

LGTM

@wmertens wmertens merged commit 3505360 into QwikDev:main Dec 30, 2023
@jakovljevic-mladen jakovljevic-mladen deleted the fix-serializeClassWithHost branch December 30, 2023 16:15
kodiakhq bot referenced this pull request in ascorbic/unpic-img Jan 7, 2024
[![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/tree/HEAD/packages/qwik)) | [`1.3.1` -> `1.3.2`](https://renovatebot.com/diffs/npm/@builder.io%2fqwik/1.3.1/1.3.2) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@builder.io%2fqwik/1.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@builder.io%2fqwik/1.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@builder.io%2fqwik/1.3.1/1.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@builder.io%2fqwik/1.3.1/1.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

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

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

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

##### What's Changed

-   docs: update portal cookbook with solved problems by [@&#8203;thejackshelton](https://togithub.com/thejackshelton) in [https://github.com/BuilderIO/qwik/pull/5600](https://togithub.com/BuilderIO/qwik/pull/5600)
-   chore: Add notice about service worker usage by [@&#8203;nelsonprsousa](https://togithub.com/nelsonprsousa) in [https://github.com/BuilderIO/qwik/pull/5606](https://togithub.com/BuilderIO/qwik/pull/5606)
-   fix: Bun integration env.get is undefined by [@&#8203;phyrog](https://togithub.com/phyrog) in [https://github.com/BuilderIO/qwik/pull/5601](https://togithub.com/BuilderIO/qwik/pull/5601)
-   fix(insights): form errors by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5607](https://togithub.com/BuilderIO/qwik/pull/5607)
-   fix(qwik-insights): fix up create application form by [@&#8203;iamriajul](https://togithub.com/iamriajul) in [https://github.com/BuilderIO/qwik/pull/5573](https://togithub.com/BuilderIO/qwik/pull/5573)
-   docs(layout): Add "Slot" import by [@&#8203;HenriqueLimas](https://togithub.com/HenriqueLimas) in [https://github.com/BuilderIO/qwik/pull/5612](https://togithub.com/BuilderIO/qwik/pull/5612)
-   fix(tests): fix typos by [@&#8203;maiieul](https://togithub.com/maiieul) in [https://github.com/BuilderIO/qwik/pull/5616](https://togithub.com/BuilderIO/qwik/pull/5616)
-   fix: Correct qwik types by [@&#8203;mhevery](https://togithub.com/mhevery) in [https://github.com/BuilderIO/qwik/pull/5623](https://togithub.com/BuilderIO/qwik/pull/5623)
-   fix(docs): add missing cookbook section by [@&#8203;gioboa](https://togithub.com/gioboa) in [https://github.com/BuilderIO/qwik/pull/5626](https://togithub.com/BuilderIO/qwik/pull/5626)
-   fix(tailwind starter): switch from cjs to esm to support vite 5 by [@&#8203;thejackshelton](https://togithub.com/thejackshelton) in [https://github.com/BuilderIO/qwik/pull/5635](https://togithub.com/BuilderIO/qwik/pull/5635)
-   docs: fix up markdown rendering by [@&#8203;ValentinBossi](https://togithub.com/ValentinBossi) in [https://github.com/BuilderIO/qwik/pull/5532](https://togithub.com/BuilderIO/qwik/pull/5532)
-   refactor(insights): improve consistency  by [@&#8203;iamriajul](https://togithub.com/iamriajul) in [https://github.com/BuilderIO/qwik/pull/5609](https://togithub.com/BuilderIO/qwik/pull/5609)
-   docs: improve getting started steps by [@&#8203;shwosner](https://togithub.com/shwosner) in [https://github.com/BuilderIO/qwik/pull/5620](https://togithub.com/BuilderIO/qwik/pull/5620)
-   docs: improve eslint loader msg + add cookbook example by [@&#8203;gioboa](https://togithub.com/gioboa) in [https://github.com/BuilderIO/qwik/pull/5591](https://togithub.com/BuilderIO/qwik/pull/5591)
-   fix(ssr): slot subscribers by [@&#8203;wmertens](https://togithub.com/wmertens) in [https://github.com/BuilderIO/qwik/pull/5608](https://togithub.com/BuilderIO/qwik/pull/5608)
-   docs: create NavLink cookbook example by [@&#8203;Adbib](https://togithub.com/Adbib) in [https://github.com/BuilderIO/qwik/pull/5621](https://togithub.com/BuilderIO/qwik/pull/5621)
-   fix(tailwind): fix prettier config type by [@&#8203;shairez](https://togithub.com/shairez) in [https://github.com/BuilderIO/qwik/pull/5641](https://togithub.com/BuilderIO/qwik/pull/5641)
-   docs: add Node Docker deploy example by [@&#8203;nelsonprsousa](https://togithub.com/nelsonprsousa) in [https://github.com/BuilderIO/qwik/pull/5605](https://togithub.com/BuilderIO/qwik/pull/5605)
-   docs(cookbook): font optimization guide by [@&#8203;thejackshelton](https://togithub.com/thejackshelton) in [https://github.com/BuilderIO/qwik/pull/5645](https://togithub.com/BuilderIO/qwik/pull/5645)
-   fix: rendering ssr and csr for value="" by [@&#8203;wmertens](https://togithub.com/wmertens) in [https://github.com/BuilderIO/qwik/pull/5642](https://togithub.com/BuilderIO/qwik/pull/5642)
-   fix: remove unnecessary whitespace when handling classes by [@&#8203;jakovljevic-mladen](https://togithub.com/jakovljevic-mladen) in [https://github.com/BuilderIO/qwik/pull/5648](https://togithub.com/BuilderIO/qwik/pull/5648)
-   fix(jsx): dynamic DOM element props by [@&#8203;wmertens](https://togithub.com/wmertens) in [https://github.com/BuilderIO/qwik/pull/5650](https://togithub.com/BuilderIO/qwik/pull/5650)
-   refactor(jsx): tiny improvement by [@&#8203;wmertens](https://togithub.com/wmertens) in [https://github.com/BuilderIO/qwik/pull/5654](https://togithub.com/BuilderIO/qwik/pull/5654)
-   feat: add `skipConfirmation` to cli add command by [@&#8203;shairez](https://togithub.com/shairez) in [https://github.com/BuilderIO/qwik/pull/5655](https://togithub.com/BuilderIO/qwik/pull/5655)
-   chore: 1.3.2 by [@&#8203;shairez](https://togithub.com/shairez) in [https://github.com/BuilderIO/qwik/pull/5661](https://togithub.com/BuilderIO/qwik/pull/5661)

##### New Contributors

-   [@&#8203;phyrog](https://togithub.com/phyrog) made their first contribution in [https://github.com/BuilderIO/qwik/pull/5601](https://togithub.com/BuilderIO/qwik/pull/5601)
-   [@&#8203;HenriqueLimas](https://togithub.com/HenriqueLimas) made their first contribution in [https://github.com/BuilderIO/qwik/pull/5612](https://togithub.com/BuilderIO/qwik/pull/5612)
-   [@&#8203;ValentinBossi](https://togithub.com/ValentinBossi) made their first contribution in [https://github.com/BuilderIO/qwik/pull/5532](https://togithub.com/BuilderIO/qwik/pull/5532)
-   [@&#8203;shwosner](https://togithub.com/shwosner) made their first contribution in [https://github.com/BuilderIO/qwik/pull/5620](https://togithub.com/BuilderIO/qwik/pull/5620)

**Full Changelog**: QwikDev/qwik@v1.3.1...v1.3.2

</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:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMjEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEyMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMP: styling 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.

None yet

3 participants