feat: bubble functional state updaters through lift#150
Merged
Conversation
- useProperty now passes original updater function in ev.detail.updater - lift passes updater fn to parent setter so it resolves against parent state - Notify first, compute after: if defaultPrevented, skip local computation - Add init/commit/updater/notify methods replacing updateProp - Event detail now includes both value and updater fields - Dedup direct values before notify (functional updaters always notify) Fixes rapid-fire functional updater calls producing incorrect state when used with lift, because the child's local state was blocked by preventDefault() and hadn't yet been updated by parent re-render. Closes #149
✅ Deploy Preview for pionjs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: aaa59cb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
updater now always dispatches the change event, even for unchanged direct values. This ensures lift always receives the updater function. The commit method still handles deduplication via Object.is to avoid redundant host writes.
…il.value - resolve() computes previousValue, value, updater from valueOrUpdater - notify() takes resolved value + optional updater, always populates ev.detail.value with the resolved value (not undefined for functions) - updater() orchestrates: resolve → notify → if prevented return → dedup+write - ev.detail.value is always the resolved value, backward compatible - ev.detail.updater carries original function for lift - Always dispatch event (no Object.is short-circuit before notify)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When
liftis used to delegate property ownership to a parent component, rapid-fire calls touseProperty's setter with functional updaters produce incorrect state. Each call reads stalehost[property]becauselift'spreventDefault()blocks the local write, and the parent hasn't re-rendered yet. See #149 for full analysis.Solution
Notify first, compute after. If
defaultPrevented(i.e.,liftintercepted), skip the local write. If not prevented, commit the resolved value.Bubble the original updater function in
ev.detail.updatersoliftcan pass it to the parent's setter. The parent's setter is itself ausePropertyupdater that callsresolve()against its ownhost[property]— the authoritative state. Meanwhile,ev.detail.valuealways contains the locally-resolved value for backward compatibility with non-lift listeners.Method breakdown
resolve(valueOrUpdater)[previousValue, value, updater]. Resolves functions againsthost[property].notify(value, updater?){ value, updater, path }in detail.valueis always the resolved value. Returns event.updater(valueOrUpdater)updateProp,init, andcommitremoved — replaced byresolve+notify+ inline dedup/write.Event detail
ev.detailnow always contains:value: the resolved value (computed againsthost[property]). Always present, neverundefined.updater: the original updater function for functional calls,undefinedfor direct values. Used byliftto resolve against parent state.path: unchanged.Flow for rapid-fire calls with lift
Each call resolves correctly at the parent level because
liftpasses the updater function, not the locally-resolved value.Backward Compatibility
ev.detail.value: always the resolved value. Previously was the resolved value for direct calls only. Non-lift listeners that readev.detail.valueget correct values for both direct and functional updaters.ev.detail.updater: new optional field — only used bylift.ev.detail.path: unchanged.Breaking changes
useProperty.updatePropremoved (was internal, not part of public API)useProperty.initremoved (was internal, not part of public API)updaterfield alongsidevalueev.detail.valueis always the resolved value (wasundefinedfor functional updaters)liftnow passesev.detail.updater ?? ev.detail.valueto setter (wasev.detail.value)usePropertysetter now always dispatches a change event, even if the value is unchangedTests
94 tests passing (89 existing + 5 new):
ev.detail.valueis resolved (9, not undefined) andev.detail.updateris the functionev.detail.valueis the value andev.detail.updateris undefinedsetItems((items) => [...items, x])calls producea,b,c