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.

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

feat: add basic support for ListSignal #2775

Merged
merged 44 commits into from
Oct 14, 2024
Merged

Conversation

taefi
Copy link
Contributor

@taefi taefi commented Sep 27, 2024

Description

This add the basic support for ListSignal<T> to be exposed in Java @BrowserCallable annotated services, and to be consumed in client code to do the following operations:

  • add an item to the end of the list: insertLast
  • remove an item from them list (any item): remove

Note that, each item in a ListSignal<T> is a ValueSignal<T>, that means you can change each individaal value and the changes would be propagated to all the clients subscribed to that ListSignal and its internal ValueSignals.

Fixes #2654

Aiming for the smaller PRs, the generator support for ListSignal type will be added in the subsequent PRs.
In the meantime, to use a service method that exposes a ListSignal instance, a manual step is needed for adding a TS service that acts instead of the "yet to be generated service".

As an example, having a service in Java like this:

package com.example.chat.services;

import com.vaadin.flow.server.auth.AnonymousAllowed;
import com.vaadin.hilla.BrowserCallable;
import com.vaadin.hilla.signals.ListSignal;

@AnonymousAllowed
@BrowserCallable
public class ChatService {
  private final ListSignal<String> chatChannel = new ListSignal<>(String.class);

  public ListSignal<String> chatChannelSignal() {
    return chatChannel;
  }
}

you need to manually add the following code to utilize the backend ListSignal:

(assume you store the following TS service in Frontend/temp/ChatService.ts)

import { ListSignal as ListSignal_1 } from "@vaadin/hilla-react-signals";
import type Message_1 from "Frontend/generated/com/example/chat/services/ChatService/Message.js";
import client_1 from "../generated/connect-client.default.js";
function chatChannelSignal_1(): ListSignal_1<Message_1> {
    return new ListSignal_1<Message_1> ( { client: client_1, endpoint: "ChatService", method: "chatChannelSignal" });
}
export { chatChannelSignal_1 as chatChannelSignal };

then in your chat.tsx, you use it like this:

import { type ListSignal, useSignal } from "@vaadin/hilla-react-signals";
// other imports
import * as ChatService from "Frontend/temp/ChatService.js";
import type Message from "Frontend/generated/com/example/chat/services/ChatService/Message.js";

const chatChannel: ListSignal<Message> = ChatService.chatChannelSignal();

export default function chat() {
  const username = useSignal('');
  const usernameLocked = useSignal(false);
  const newMessage = useSignal('');

  return (
      <VerticalLayout theme='padding'>
        <h3>Chat</h3>
        <ul>
          {chatChannel.value.length === 0 ? <li>No messages yet...</li> :
            chatChannel.value.map((message, index) => (
              <li key={index}>{message.value.author}: {message.value.text}</li>
            ))}
        </ul>
        <TextField label='Username' disabled={usernameLocked.value} value={username.value}
                   onValueChanged={(e) => username.value = e.detail.value}/>
        <TextArea value={newMessage.value} placeholder="Type in your message and press send..."
                  onValueChanged={(e => newMessage.value = e.detail.value)}/>
        <Button onClick={() => {
          chatChannel.insertLast({text: newMessage.value, author: username.value});
          newMessage.value = '';
          usernameLocked.value = true;
        }} disabled={newMessage.value === ''}>Send</Button>
      </VerticalLayout>
  );
}

I skipped demonstrating the usage of remove operation for brevity, but since it accepts a instance of ValueSignal<T>, it is clear that chatChannel.value[i] can be passed to it to be removed.

Type of change

  • Bugfix
  • Feature

Checklist

  • I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

Additional for Feature type of change

  • Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

This changes the way updates are sent to the clients
after the acceptance/rejection of an event on the
server. With this applied, the REJECT event is removed
and SNAPSHOT event containing the current state of the
server-side signal only when (re)subscribing to a signal
and not after each update. The events received by the
server regarding each update to a signal is propagated
to the clients with a boolean value that indicates the
acceptance/rejection of the operation so clients decide
to discard/apply/confirm the operation on their local
value.
This unifies the way with how List signals are going
to work: to get snapshot events with the current state
on the server only when subscribing signals (which can
be a long list of values), and then getting updates of
individual operations for subsequent updates, one at a
time.
This extract the Signal abstraction out of ValueSignal so that the ListSignal implementation can inherit the common functionalities.

Part of #2654
Copy link

codecov bot commented Sep 27, 2024

Codecov Report

Attention: Patch coverage is 99.01961% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.49%. Comparing base (3b56dad) to head (f08fb06).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
packages/ts/react-signals/src/ValueSignal.ts 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2775      +/-   ##
==========================================
+ Coverage   92.28%   92.49%   +0.20%     
==========================================
  Files          81       83       +2     
  Lines        2683     2771      +88     
  Branches      688      717      +29     
==========================================
+ Hits         2476     2563      +87     
  Misses        156      156              
- Partials       51       52       +1     
Flag Coverage Δ
unittests 92.49% <99.01%> (+0.20%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Base automatically changed from taefi/refactor/extract-signal-abstraction to main September 27, 2024 12:31
@taefi taefi marked this pull request as ready for review October 8, 2024 18:21
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
12.9% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@platosha platosha merged commit 0b5dedc into main Oct 14, 2024
14 of 15 checks passed
@platosha platosha deleted the taefi/feat/add-list-signal branch October 14, 2024 15:35
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Hilla 24.6.0.alpha1 and is also targeting the upcoming stable 24.6.0 version.

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.

[Full-stack Signals] Add basic support for List signals
3 participants