refactor: rename WritableSignal.value() to set() #23305
Draft
Conversation
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
Renames ValueSignal class to SharedValueSignal to clarify that this is a shared signal (serializable, cluster-capable) as opposed to local signals which will be introduced in future commits. This is part of the incremental signals restructuring effort to distinguish between: - Local signals: non-serializable, UI-local state management - Shared signals: serializable, cluster-capable state sharing Changes: - Renamed ValueSignal.java -> SharedValueSignal.java - Updated all references in signals module (5 main files, 7 test files) - Updated all references in flow-server module (8 main files, 17 test files) - Updated subclasses: NumberSignal, NodeSignal - Updated dependent classes: ListSignal, MapSignal - All tests pass: 446 signals tests, 214 flow-server signal tests Part of incremental signals restructuring. Phase 1 of 4.
… package Move core interfaces to com.vaadin.signals.core package: - Signal.java - WritableSignal.java - BindingActiveException.java Update all imports across signals and flow-server modules.
Move all shared signal classes from com.vaadin.signals to com.vaadin.signals.shared: - AbstractSignal - SharedValueSignal - SharedNumberSignal - SharedNodeSignal - SharedListSignal - SharedMapSignal - SignalUtils (helper class) Also move corresponding test files to the shared package. Update all imports across signals and flow-server modules.
Move ReferenceSignal to the local package and rename to ValueSignal. Local signals are intended for UI-local state only and do not participate in clustering. The local package provides non-serializable signals that are simpler than shared signals and don't use JSON serialization.
Update javadoc examples in binding methods to use the simpler local ValueSignal instead of SharedValueSignal. ValueSignal is more appropriate for introductory documentation as it's simpler and doesn't require understanding shared/distributed state concepts. Both signal types work with the binding methods since they accept the Signal<T> interface.
Update test classes to use the simpler local ValueSignal instead of SharedValueSignal. These tests verify UI binding behavior which is local to the UI and doesn't require shared/distributed state.
… root package Move core interfaces back from com.vaadin.signals.core to com.vaadin.signals. A separate core package adds no value when the root package works just as well for these top-level types.
Move general-purpose classes (ComputedSignal, Transaction, Effect, TransientListener, UsageTracker) to the root signals package and shared-signal implementation classes (SignalTree, StagedTransaction, TreeRevision, etc.) to shared/impl. This eliminates the impl/ package and colocates classes with their logical grouping.
Update 7 flow-tests view files to import ValueSignal from com.vaadin.signals.local instead of com.vaadin.signals. Fix two broken javadoc references: AbstractSignal#peek() in Signal.java and Signal#runInTransaction in SharedValueSignal.java.
Local ValueSignal stores raw objects, so signal.peek() returns Person records rather than Maps. Update the casts in bindListProperty and bindMapProperty reattach tests accordingly.
The method name set() better communicates the intent of the operation and avoids confusion with the value() getter inherited from Signal.
Format Checker Report
Here is the list of files with format issues in your PR: |
|
Test Results1 325 files ±0 1 325 suites ±0 1h 15m 46s ⏱️ + 1m 29s Results for commit 653a03f. ± Comparison against base commit 8df7646. This pull request removes 437 and adds 437 tests. Note that renamed tests count towards both. |
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.
The method name set() better communicates the intent of the operation
and avoids confusion with the value() getter inherited from Signal.