Skip to content

Conversation

@Legioth
Copy link
Member

@Legioth Legioth commented Jan 12, 2026

The factory interface makes sense only with IOC to inject an implementation without caring about what implementation that is. We now only support using through the IN_MEMORY_SHARED constant which defeats the purpose since you're then anyways referencing a specific implementation throughout application code.

What we should really do is to provide the in-memory factory as a Spring/CDI bean by default if no other factory bean is defined in the application. But at the same time, there's little point in doing that now before we have any meaningful alternative implementation to use so that we could verify that it actually works to just swap out the implementation without additional changes. Because of this, we should remove the abstraction now and then add it back later when we add clustering support.

The factory interface makes sense only with IOC to inject an
implementation without caring about what implementation that is. We now
only support using through the `IN_MEMORY_SHARED` constant which defeats
the purpose since you're then anyways referencing a specific
implementation throughout application code.

What we should really do is to provide the in-memory factory as a
Spring/CDI bean by default if no other factory bean is defined in the
application. But at the same time, there's little point in doing that
now before we have any meaningful alternative implementation to use so
that we could verify that it actually works to just swap out the
implementation without additional changes. Because of this, we should
remove the abstraction now and then add it back later when we add
clustering support.
@sonarqubecloud
Copy link

@github-actions
Copy link

Test Results

1 310 files   -  1  1 310 suites   - 1   1h 13m 33s ⏱️ - 1m 17s
9 283 tests  - 15  9 215 ✅  - 15  68 💤 ±0  0 ❌ ±0 
9 737 runs   - 22  9 661 ✅  - 22  76 💤 ±0  0 ❌ ±0 

Results for commit 6363690. ± Comparison against base commit cda083a.

This pull request removes 15 tests.
com.vaadin.signals.SignalFactoryTest ‑ exclusive_twoInstancesSameName_separateInstances
com.vaadin.signals.SignalFactoryTest ‑ list_newSignal_isListSignalWithRightType
com.vaadin.signals.SignalFactoryTest ‑ map_newSignal_isListSignalWithRightType
com.vaadin.signals.SignalFactoryTest ‑ number_existingWithDefaultValue_usesOldValue
com.vaadin.signals.SignalFactoryTest ‑ number_newSignal_isNumberSignal
com.vaadin.signals.SignalFactoryTest ‑ number_newWithDefaultValue_usesDefaultValue
com.vaadin.signals.SignalFactoryTest ‑ shared_clear_separateInstances
com.vaadin.signals.SignalFactoryTest ‑ shared_removeName_separateInstances
com.vaadin.signals.SignalFactoryTest ‑ shared_sameNameDifferentTypes_dataIsShared
com.vaadin.signals.SignalFactoryTest ‑ shared_twoInstancesDifferentName_separateInstances
…

@Artur- Artur- merged commit c9de0d9 into main Jan 12, 2026
31 checks passed
@Artur- Artur- deleted the removeSignalFactory branch January 12, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants