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

Value changes many time in instantly when using the multiple move method of LiveList quickly #1518

Open
jiwon79 opened this issue Mar 8, 2024 · 3 comments
Labels
bug Something isn't working Custom App

Comments

@jiwon79
Copy link

jiwon79 commented Mar 8, 2024

Describe the bug
Value change instantly when using the multiple LiveList move method quickly

To Reproduce

  1. fork liveblocks todo list
  1. add API_KEY in liveblocks.config.ts
  2. change index.ts content like following
import { useState, useMemo } from "react";
import {
  RoomProvider,
  useOthers,
  useUpdateMyPresence,
  useStorage,
  useMutation,
} from "../liveblocks.config";
import "@liveblocks/react";
import { LiveList, LiveObject } from "@liveblocks/client";
import { useRouter } from "next/router";
import { ClientSideSuspense } from "@liveblocks/react";

function WhoIsHere() {
  const userCount = useOthers((others) => others.length);

  return (
    <div className="who_is_here">There are {userCount} other users online</div>
  );
}

function SomeoneIsTyping() {
  const someoneIsTyping = useOthers((others) =>
    others.some((other) => other.presence.isTyping)
  );

  return (
    <div className="someone_is_typing">
      {someoneIsTyping ? "Someone is typing..." : ""}
    </div>
  );
}

let i = 0;

function Example() {
  const [selected, setSelected] = useState("")
  const [draft, setDraft] = useState("");
  const updateMyPresence = useUpdateMyPresence();
  const todos = useStorage((root) => root.todos);

  const moveFront = useMutation(({ storage }) => {
    const index = todos.findIndex((todo) => todo.id === selected)
    console.log('@index', todos, index)
    if (index !== -1) {
      storage.get("todos").move(index, Math.max(index - 1, 0))
    }
  }, [todos, selected]);

  const moveBack = useMutation(({ storage }) => {
    const index = todos.findIndex((todo) => todo.id === selected)
    console.log('@index', todos, index)
    if (index !== -1) {
      storage.get("todos").move(index, Math.min(index + 1, todos.length))
    }
  }, [todos, selected]);

  const addTodo = useMutation(({ storage }, text) => {
    const id = `${i++}`
    storage.get("todos").push(new LiveObject({ id, text }));
  }, []);

  const toggleTodo = useMutation(({ storage }, index) => {
    const todo = storage.get("todos").get(index);
    todo?.set("checked", !todo.get("checked"));
  }, []);

  const deleteTodo = useMutation(({ storage }, index) => {
    storage.get("todos").delete(index);
  }, []);

  return (
    <div className="container">
      <WhoIsHere />
      <input
        type="text"
        placeholder="What needs to be done?"
        value={draft}
        onChange={(e) => {
          setDraft(e.target.value);
          updateMyPresence({ isTyping: true });
        }}
        onKeyDown={(e) => {
          if (draft && e.key === "Enter") {
            updateMyPresence({ isTyping: false });
            addTodo(draft);
            setDraft("");
          }
        }}
        onBlur={() => updateMyPresence({ isTyping: false })}
      />
      <SomeoneIsTyping />
      <p>selected : {selected}</p>
      <button onClick={() => moveFront()}>move front</button>
      <button onClick={() => moveBack()}>move back</button>
      {todos.map((todo, index) => {
        return (
          <div key={index} className="todo_container">
            <div className="todo" onClick={() => toggleTodo(index)}>
              <span
                style={{
                  cursor: "pointer",
                  textDecoration: todo.checked ? "line-through" : undefined,
                }}
              >
                {todo.id} - {todo.text}
              </span>
            </div>
            <button className="delete_button" onClick={() => deleteTodo(index)}></button>
            <button
              style={{backgroundColor: selected === todo.id ? 'red' : 'grey'}}
              onClick={() => setSelected(todo.id)}
              >selecte</button>
          </div>
        );
      })}
    </div>
  );
}

function Loading() {
  return (
    <div className="loading">
      <img src="https://liveblocks.io/loading.svg" alt="Loading" />
    </div>
  );
}

export default function Page() {
  const roomId = useOverrideRoomId("nextjs-todo-list-v2");

  return (
    <RoomProvider
      id={roomId}
      initialPresence={{
        isTyping: false,
      }}
      initialStorage={{ todos: new LiveList() }}
    >
      <ClientSideSuspense fallback={<Loading />}>
        {() => <Example />}
      </ClientSideSuspense>
    </RoomProvider>
  );
}

export async function getStaticProps() {
  const API_KEY = process.env.NEXT_PUBLIC_LIVEBLOCKS_PUBLIC_KEY;
  const API_KEY_WARNING = process.env.CODESANDBOX_SSE
    ? `Add your public key from https://liveblocks.io/dashboard/apikeys as the \`NEXT_PUBLIC_LIVEBLOCKS_PUBLIC_KEY\` secret in CodeSandbox.\n` +
      `Learn more: https://github.com/liveblocks/liveblocks/tree/main/examples/nextjs-live-cursors#codesandbox.`
    : `Create an \`.env.local\` file and add your public key from https://liveblocks.io/dashboard/apikeys as the \`NEXT_PUBLIC_LIVEBLOCKS_PUBLIC_KEY\` environment variable.\n` +
      `Learn more: https://github.com/liveblocks/liveblocks/tree/main/examples/nextjs-live-cursors#getting-started.`;

  if (!API_KEY) {
    console.warn(API_KEY_WARNING);
  }

  return { props: {} };
}

/**
 * This function is used when deploying an example on liveblocks.io.
 * You can ignore it completely if you run the example locally.
 */
function useOverrideRoomId(roomId: string) {
  const { query } = useRouter();
  const overrideRoomId = useMemo(() => {
    return query?.roomId ? `${roomId}-${query.roomId}` : roomId;
  }, [query, roomId]);

  return overrideRoomId;
}

Expected behavior

  • The number of times the todos changes is equal to the number of times the move method is used.
  • However todos change instantly, the number of times the todoschanges is more than method used count.

Illustrations

2024-03-08.11.14.17.mov

Environment (please complete the following information):

  • Liveblocks packages and packages version
    • @liveblocks/react@1.10.0
    • @liveblocks/client@0.15.1
  • Browser's version : chrome, codesandbox
@jiwon79 jiwon79 added bug Something isn't working triage needed The issue needs to be reviewed by the team labels Mar 8, 2024
@jiwon79 jiwon79 changed the title Value changes instantly when using the multiple LiveList move method quickly Value changes many time in instantly when using the multiple move method of LiveList quickly Mar 11, 2024
@GuillaumeSalles
Copy link
Contributor

Hi @jiwon79,

First of all, thank you for the detailed bug report and patience.

Liveblocks packages and packages version
@liveblocks/react@1.10.0
@liveblocks/client@0.15.1

I believe this issue is also present on the latest version, but can you confirm you see the same behavior after upgrading? We recommend having the same version for both @liveblocks/react and @liveblocks/client.

Unfortunately, we have a few LiveList issues including that one that requires a deeper refactoring to be able to fix them properly. We have plans to do that, but not in the short term. Could you explain what is your use case and why consecutive move are frequently done in your product? Maybe there is a workaround that we can help you find

@enk0de
Copy link

enk0de commented Mar 28, 2024

Hi! Thank you for response. I am on the same team with @jiwon79


We recommend having the same version for both @liveblocks/react and @liveblocks/client.

A: We are already using same version for @liveblocks/react, @liveblocks/client.

Why consecutive move are frequently done in your product

A: We are creating a Figma-like Design Tool. We have a "Stack" feature, in which user can move index element in Stack. Then when user moving element index in Stack quite fastly, blinking issue appears.


Did you know that there are also serious history management issues? History Undo/Redo doesn't work well. If you go back to the days before history.undo() immediately after creating an item, all history records will be deleted, or you can use the History. This problem occurs in liveblocks official demo page too (https://liveblocks.io/document/storage)

@enk0de
Copy link

enk0de commented Apr 3, 2024

i found that the logic at case of ServerMsgCode.UPDATE_STORAGE in room.ts_handleServerMessage causes the local state to mutate. Shouldn't updates be ignored If both remote event trigger source and the client are same? (updating storage is initiated by the client itself)

          case ServerMsgCode.UPDATE_STORAGE: {
            const applyResult = applyOps(message.ops, false);
            for (const [key, value] of applyResult.updates.storageUpdates) {
              updates.storageUpdates.set(
                key,
                mergeStorageUpdates(updates.storageUpdates.get(key), value)
              );
            }
            break;
          }

@adigau adigau added the public repo label Apr 15, 2024 — with Linear
@adigau adigau removed triage needed The issue needs to be reviewed by the team public repo labels Apr 15, 2024
@adigau adigau added the 🎪 Room label Apr 24, 2024 — with Linear
@adigau adigau added the Engineering label May 9, 2024 — with Linear
@adigau adigau removed the Engineering label May 9, 2024
@adigau adigau removed the 🎪 Room label May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Custom App
Projects
None yet
Development

No branches or pull requests

4 participants