-
Notifications
You must be signed in to change notification settings - Fork 438
feat: signals implementation v1 #3963
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
Changes from 4 commits
4a0e5f7
f7f658d
0a08b45
66f885b
65fa1cb
8037a6c
7b3582f
92413fc
fcdae16
cf3bafd
b06528b
f315852
4ec0439
3c78f17
e3b6e82
2fa1693
1565a04
6fde22a
3c6aa00
1565db7
6360bb6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -28,11 +28,37 @@ export function componentValueMutated(vm: VM, key: PropertyKey) { | |||
| } | ||||
| } | ||||
|
|
||||
| export function componentValueObserved(vm: VM, key: PropertyKey) { | ||||
| export function componentValueObserved(vm: VM, key: PropertyKey, target: any = {}) { | ||||
| // On the server side, we don't need mutation tracking. Skipping it improves performance. | ||||
| if (process.env.IS_BROWSER) { | ||||
| valueObserved(vm.component, key); | ||||
| } | ||||
|
|
||||
| // Putting this here for now, the idea is to subscribe to a signal when there is an active template reactive observer. | ||||
| // This would indicate that: | ||||
| // 1. The template is currently being rendered | ||||
| // 2. There was a call to a getter bound to the LWC class | ||||
| // With this information we can infer that it is safe to subscribe the re-render callback to the signal, which will | ||||
| // mark the VM as dirty and schedule rehydration. | ||||
| if ( | ||||
| target && | ||||
| typeof target === 'object' && | ||||
| 'value' in target && | ||||
| 'subscribe' in target && | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
No need since you're already checking it's a function below.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nolanlawson typescript is complaining when I do |
||||
| typeof target.subscribe === 'function' | ||||
| ) { | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These conditions look right to me for now. Sometime after this PR lands, we may want to replace the last three conditions with something like |
||||
| if (vm.tro.isObserving()) { | ||||
| try { | ||||
| // In a future optimization, rather than re-render the entire VM we could use fine grained reactivity here | ||||
| // to only re-render the part of the DOM that has been changed by the signal. | ||||
| // jtu-todo: this will subscribe multiple functions since the callback is always different, look for a way to deduplicate this | ||||
| const unsubscribe = target.subscribe(() => vm.tro.notify()); | ||||
| vm.tro.link(unsubscribe); | ||||
| } catch (e) { | ||||
| // throw away for now | ||||
| } | ||||
| } | ||||
| } | ||||
| } | ||||
|
|
||||
| export function createReactiveObserver(callback: CallbackFunction): ReactiveObserver { | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| /* | ||
| * Copyright (c) 2023, salesforce.com, inc. | ||
| * All rights reserved. | ||
| * SPDX-License-Identifier: MIT | ||
| * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT | ||
| */ | ||
|
|
||
| export type OnUpdate = () => void; | ||
| export type Unsubscribe = () => void; | ||
|
|
||
| export interface Signal<ValueShape> { | ||
| get value(): ValueShape; | ||
| subscribe(onUpdate: OnUpdate): Unsubscribe; | ||
| } | ||
|
|
||
| export abstract class SignalBaseClass<ValueShape> implements Signal<ValueShape> { | ||
| abstract get value(): ValueShape; | ||
|
|
||
| private subscribers: Set<OnUpdate> = new Set(); | ||
|
|
||
| subscribe(onUpdate: OnUpdate) { | ||
| this.subscribers.add(onUpdate); | ||
| return () => { | ||
| this.subscribers.delete(onUpdate); | ||
| }; | ||
| } | ||
|
|
||
| protected notify() { | ||
| for (const subscriber of this.subscribers) { | ||
| subscriber(); | ||
| } | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we discussed, I'd love for this to be moved to a new top-level
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
Uh oh!
There was an error while loading. Please reload this page.