tutorialkit icon indicating copy to clipboard operation
tutorialkit copied to clipboard

tutorialStore onDocumentChanged unexpected behaviour

Open so0k opened this issue 1 year ago • 1 comments

Describe the bug

using the tutorialStore.onDocumentChanged triggers callback immediately because the nanostores subscribe callback seems to work that way.

For example:

    const terminal = tutorialStore.terminalConfig.value!.panels[1];
    const filePath = '/foo';
    tutorialStore.onDocumentChanged(filePath, (document) => {
       terminal.write(`${document.filePath} changed\n`);
    });

Expected comparison between old and new document to confirm changes happened

    const terminal = tutorialStore.terminalConfig.value!.panels[1];
    const filePath = '/foo';
    const stopListening = tutorialStore.documents.listen((newDocuments, oldDocuments) => {
      if (!newDocuments[filePath] || !oldDocuments || !oldDocuments[filePath]) {
        return;
      }
      const oldDocument = oldDocuments[filePath];
      const newDocument = newDocuments[filePath];
      if (oldDocument.value !== newDocument.value) {
        queueMicrotask(() => {
          terminal.write(`${newDocument.filePath} changed\n`);
          stopListening();
        });
      }
    });

Link to a StackBlitz project which shows the error

https://stackblitz.com/~/github.com/so0k/tutorialkit-terminal-writer

Steps to reproduce

  1. click "Test" button
  2. Notice terminal shows "Document changed" when it did not

Expected behavior

  1. click "Test" Button
  2. Modify foo (i.e. use TutorialKit API to update foo)
  3. only then, notice callback is triggered

Screenshots

No response

Platform

  • TutorialKit version: 1.3.1
  • OS: local Linux, Stackblitz
  • Browser: Chrome
  • Version: 131.0.6778.204 (Official Build) (64-bit)

Additional context

  1. Consider using listen if we don't want immediate callback?
  2. Compare newValue(s) to oldValue(s) before triggering?

so0k avatar Jan 11 '25 23:01 so0k

because the nanostores subscribe callback seems to work that way.

This seems to be the root cause indeed:

store.subscribe(cb) in contrast with store.listen(cb) also call listeners immediately during the subscription.

https://github.com/nanostores/nanostores?tab=readme-ov-file#atoms

https://github.com/stackblitz/tutorialkit/blob/0e7cf3c082d4ceb298668e06a4914253327eb0c3/packages/runtime/src/store/editor.ts#L148-L173

The onDocumentChanged is not used internally by TutorialKit, it's only available via the experimental API. Fixing this should be quite risk-free.

AriPerkkio avatar Jan 13 '25 06:01 AriPerkkio