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

refactor(core): extract Group type #4798

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

wtlin1228
Copy link
Contributor

@wtlin1228 wtlin1228 commented Jul 15, 2023

Overview

What is it?

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

Description

While going over the Qwik source code, I noticed SubscriberEffect | SubscriberHost | Node type is used in many places that might be clearer if it is extracted into a standalone Group type.

I was considering to derive Group type from Subscriptions type like the following because group can only be the host of subscription.

type Group = Subscriptions[1]; // SubscriberEffect | QwikElement | Text

But that will cause a problem in the cleanupTree. So I decided not to do so in this PR.

export const cleanupTree = (
  elm: Node | VirtualElement,
  // ...
) => {
  // ❌ Type 'Node' is not assignable to type 'SubscriberEffect | QwikElement | Text'.
  subsManager.$clearSub$(elm); 
  // ...
};

Use cases and why

    1. One use case
    1. Another use case

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

@stackblitz
Copy link

stackblitz bot commented Jul 15, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@netlify
Copy link

netlify bot commented Jul 15, 2023

👷 Deploy request for qwik-insights pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 86aa061

@zanettin zanettin added WAITING FOR: team Waiting for one of the core team members to review and reply COMP: typing labels Aug 15, 2023
@wmertens wmertens changed the title refactor: extract group type refactor(core): extract Group type Nov 30, 2023
@wmertens wmertens merged commit dade997 into QwikDev:main Nov 30, 2023
20 checks passed
@wmertens
Copy link
Member

Thanks!

kodiakhq bot pushed a commit to ascorbic/unpic-img that referenced this pull request Dec 10, 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/tree/HEAD/packages/qwik)) | [`1.2.19` -> `1.3.0`](https://renovatebot.com/diffs/npm/@builder.io%2fqwik/1.2.19/1.3.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/@builder.io%2fqwik/1.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@builder.io%2fqwik/1.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@builder.io%2fqwik/1.2.19/1.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@builder.io%2fqwik/1.2.19/1.3.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

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

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

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

#### Breaking Changes

-   When passing a `Signal` into `track()` directly (instead of passing a function), it returns the signal value instead of the signal itself. This behavior was already present when running inside a `useResource$` callback, and now it is consistent across all Tasks.

#### What's Changed

-   chore: Vite 5 by [@&#8203;wmertens](https://togithub.com/wmertens) in [QwikDev/qwik#5451
-   feat: add qwik/no-use-visible-task eslint rule by [@&#8203;gioboa](https://togithub.com/gioboa) in [QwikDev/qwik#5455
-   chore(insights): remove unnecessary log by [@&#8203;gioboa](https://togithub.com/gioboa) in [QwikDev/qwik#5461
-   fix: add example context to docs by [@&#8203;dotarjun](https://togithub.com/dotarjun) in [QwikDev/qwik#5467
-   feat(qwik-city): allow customizing SVGO options of image plugin by [@&#8203;hbendev](https://togithub.com/hbendev) in [QwikDev/qwik#5407
-   docs: fix typo by [@&#8203;ulic75](https://togithub.com/ulic75) in [QwikDev/qwik#5472
-   docs: fix typo by [@&#8203;Adbib](https://togithub.com/Adbib) in [QwikDev/qwik#5481
-   fix(core): Support JSX in signals by [@&#8203;mhevery](https://togithub.com/mhevery) in [QwikDev/qwik#5442
-   docs(FAQ): - lazy-loading on user interaction & speculative module fetching by [@&#8203;maiieul](https://togithub.com/maiieul) in [QwikDev/qwik#5488
-   docs(faq): add link to typescript by [@&#8203;maiieul](https://togithub.com/maiieul) in [QwikDev/qwik#5487
-   fix: disable Vite modulepreload by [@&#8203;gioboa](https://togithub.com/gioboa) in [QwikDev/qwik#5493
-   docs(faq): fix typos and improve the wording of some sentences by [@&#8203;maiieul](https://togithub.com/maiieul) in [QwikDev/qwik#5490
-   docs: make the distinction between module-prefetching and <Link prefetch> by [@&#8203;maiieul](https://togithub.com/maiieul) in [QwikDev/qwik#5485
-   docs(showcase): add `index.app` and `wiza.co` by [@&#8203;necatikcl](https://togithub.com/necatikcl) in [QwikDev/qwik#5484
-   fix(docs): mdx interpreting title as component by [@&#8203;maiieul](https://togithub.com/maiieul) in [QwikDev/qwik#5499
-   docs(/faq#vdom): cleanup the vdom section by [@&#8203;maiieul](https://togithub.com/maiieul) in [QwikDev/qwik#5500
-   fix: revert "fix: remove cf pages stream polyfill" by [@&#8203;gioboa](https://togithub.com/gioboa) in [QwikDev/qwik#5502
-   fix(qwik-city): prefix ids of SVGs based on their path when loaded as qwik nodes by [@&#8203;hbendev](https://togithub.com/hbendev) in [QwikDev/qwik#5497
-   fix: cf pages polyfill only if needed by [@&#8203;mpeg](https://togithub.com/mpeg) in [QwikDev/qwik#5507
-   refactor(core): extract Group type by [@&#8203;wtlin1228](https://togithub.com/wtlin1228) in [QwikDev/qwik#4798
-   docs: add QwikCityMockProvider explanation by [@&#8203;mulztob](https://togithub.com/mulztob) in [QwikDev/qwik#5505
-   docs(glob-import): add documentation for import.meta.glob by [@&#8203;maiieul](https://togithub.com/maiieul) in [QwikDev/qwik#5504
-   fix: CF pages polyfill also when shimmed by [@&#8203;mpeg](https://togithub.com/mpeg) in [QwikDev/qwik#5512
-   fix(qwik): handle cypress dev server path by [@&#8203;dmitry-stepanenko](https://togithub.com/dmitry-stepanenko) in [QwikDev/qwik#5509
-   chore(cf): Fix CloudFlare build failure by [@&#8203;mhevery](https://togithub.com/mhevery) in [QwikDev/qwik#5524
-   docs: removed defunct sites by [@&#8203;mhevery](https://togithub.com/mhevery) in [QwikDev/qwik#5528
-   chore(netlify): fix netlify failure by [@&#8203;mhevery](https://togithub.com/mhevery) in [QwikDev/qwik#5527
-   docs(showcase): add juneikerc.com - blog \[prismic cms] and personal website  by [@&#8203;juneikerc](https://togithub.com/juneikerc) in [QwikDev/qwik#5523
-   fix(localization-starter): update docs and add script by [@&#8203;tzdesign](https://togithub.com/tzdesign) in [QwikDev/qwik#5494
-   chore(qwik-city): bump vite-imagetools ^5 -> ^6 by [@&#8203;birkskyum](https://togithub.com/birkskyum) in [QwikDev/qwik#5525
-   feat(insights): polish UI by [@&#8203;zanettin](https://togithub.com/zanettin) in [QwikDev/qwik#5503
-   chore: update .nvmrc by [@&#8203;gioboa](https://togithub.com/gioboa) in [QwikDev/qwik#5530
-   feat(insight): Add user auth to only see your app data. by [@&#8203;mhevery](https://togithub.com/mhevery) in [QwikDev/qwik#5517
-   chore(ci): set up CI for insights and docs by [@&#8203;mhevery](https://togithub.com/mhevery) in [QwikDev/qwik#5533
-   refactor(types): more precision by [@&#8203;wmertens](https://togithub.com/wmertens) in [QwikDev/qwik#5531
-   chore: 1.3.0 by [@&#8203;mhevery](https://togithub.com/mhevery) in [QwikDev/qwik#5535

#### New Contributors

-   [@&#8203;dotarjun](https://togithub.com/dotarjun) made their first contribution in [QwikDev/qwik#5467
-   [@&#8203;Adbib](https://togithub.com/Adbib) made their first contribution in [QwikDev/qwik#5481
-   [@&#8203;mulztob](https://togithub.com/mulztob) made their first contribution in [QwikDev/qwik#5505
-   [@&#8203;juneikerc](https://togithub.com/juneikerc) made their first contribution in [QwikDev/qwik#5523
-   [@&#8203;birkskyum](https://togithub.com/birkskyum) made their first contribution in [QwikDev/qwik#5525

**Full Changelog**: QwikDev/qwik@v1.2.19...v1.3.0

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