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.

Sign up for GitHub

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

Jump to bottom

feat(v2): new signals implementation #6443

Merged
merged 91 commits into from
Sep 21, 2024
Merged

feat(v2): new signals implementation #6443

merged 91 commits into from
Sep 21, 2024

Conversation

mhevery
Copy link
Contributor

@mhevery mhevery commented Jun 2, 2024 •
edited by Varixo
Loading

Overview

This is an exploration (may not land) the Signal implementation for something more clear / more maintainable.

Todo:

  • Figure out why the unit test complains when removing .only
  • element subscriber type
  • change all subscriber invocations to be the format that Signal2 uses
  • store support (deep)
  • (de)serialization
  • replace original signal
  • remove original signal and rename

Note about cross-container support: because:

  • containers can resume in any order
  • qwikloader only handles events, not serialization
  • versions may differ between containers

, the only way to communicate between containers is with custom events like "q-signal:my- global-name" that dispatch signal values. This would be another effect handled by a separate GlobalSignal.

@mhevery mhevery requested a review from a team as a code owner June 2, 2024 13:21
@mhevery mhevery marked this pull request as draft June 2, 2024 13:21
@cloudflare-workers-and-pages Cloudflare Workers and Pages
Copy link

cloudflare-workers-and-pages bot commented Jun 2, 2024 •
edited
Loading

Deploying qwik-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 23a0a66
Status:🚫  Build failed.

View logs

@JerryWu1234
Copy link
Contributor

image

Will this PR also fix these test problems?

@wmertens
Copy link
Member

@JerryWu1234 right that's the idea, we were having trouble getting signals to work so we decided to improve the implementation

@wmertens wmertens mentioned this pull request Jun 11, 2024
Merged
2 tasks
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jul 23, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: 02ae97a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Varixo and others added 7 commits August 31, 2024 19:04
@Varixo
fix build
36d6fec
@Varixo
add failing test
8eafb46
@Varixo
serialize ready value instead of dirty marker and fix store destructu…
d53bed9 
…ring
@Varixo
fix use-computed lazy evaluation
c6ea29d
@Varixo
replace ':' with Q_PROPS_SEPARATOR
b068ab3
@Varixo @thejackshelton @wmertens
rewrite scheduleRender function
ca8842d 
fix store deserialization without effects

Co-authored-by: Jack Shelton <thejackshelton@users.noreply.github.com>
Co-authored-by: Wout Mertens <Wout.Mertens@gmail.com>
@Varixo @thejackshelton
Allow assigning undefined as a new prop in store
fcd3e74 
Co-authored-by: Jack Shelton <thejackshelton@users.noreply.github.com>
@Varixo Varixo self-assigned this Sep 12, 2024
@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Sep 21, 2024 •
edited
Loading

Open in Stackblitz

npm i https://pkg.pr.new/@builder.io/qwik@6443

npm i https://pkg.pr.new/@builder.io/qwik-city@6443

npm i https://pkg.pr.new/eslint-plugin-qwik@6443

npm i https://pkg.pr.new/create-qwik@6443

commit: 02ae97a

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 21, 2024 •
edited
Loading

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
qwik-docs ✅ Ready (View Log) Visit Preview 02ae97a

@Varixo Varixo marked this pull request as ready for review September 21, 2024 19:46
@Varixo Varixo requested review from a team as code owners September 21, 2024 19:46
@Varixo Varixo merged commit 6c4c5b9 into build/v2 Sep 21, 2024
31 checks passed
@Varixo Varixo deleted the build/v2-signals branch September 21, 2024 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wmertens wmertens wmertens approved these changes

Assignees

@mhevery mhevery

@Varixo Varixo

Labels
None yet
Projects
Qwik Development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mhevery @JerryWu1234 @wmertens @Varixo