feat: restore documentation examples #321

stone-lyl · 2024-10-18T09:49:54Z

website playground:

website.mp4

node server playground:

node.mp4

vscode

vscode.mp4

Before	After
❌	✅

fix: resolve node server could not auto restart
- the previous nodemon couldn't auto-restart on Windows. I've switched to pm2 now.

Before	After
❌	✅

…message

vercel · 2024-10-18T09:49:58Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
data-story-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 21, 2024 8:26am

ajthinking

Appreciate 🙏 🌟

Testing pm2

Works great on mac. Tested making a change in a TS file, the server immediately restarted with change applied 👍

Possible updateDiagram bug?

Not sure when/how I triggered this. Possibly just when adding/moving nodes?

Unfolding bug

Unclear when this bug was introduced. Might be unrelated/out of scope to this PR.

Vscode broken

When creating a new diagram I noticed:

slightly longer load time (seeing spinner)
I could not run (no items shown)
when switching between files seeing this:

packages/ds-ext/src/DiagramEditorProvider.ts

packages/ds-ext/src/app/App.tsx

ajthinking · 2024-10-19T07:28:56Z

packages/ds-ext/src/app/App.tsx

-
-      // Send the message to VS Code extension
-      window.vscode.postMessage(updatedData);
+      client?.updateDiagram?.(diagram);


Do we need the safe(?) operators here? At this time, could/should we enforce a client always to be present?

ajthinking · 2024-10-19T07:34:21Z

packages/ds-ext/src/messageHandlers/onUpdateDiagram.ts

@@ -5,6 +5,6 @@ export const onUpdateDiagram: MessageHandler = async ({
  event,
  document,
 }) => {
-  document.update(new TextEncoder().encode(event.diagram));
+  document.update(new TextEncoder().encode(JSON.stringify(event.diagram)));


This is quite a change, might be related to section "Vscode broken"?

Yes, I've standardized the data format returned by the various servers (jsServer, socketServer, and vscodeServer). Therefore, we need to recreate the .diagram.json file in ds-ext 😂

ajthinking · 2024-10-19T07:34:40Z

packages/nodejs/package.json

@@ -16,7 +16,7 @@
    "test": "yarn run -T vitest",
    "build": "tsc",
    "release": "yarn run -T release-it",
-    "watch:server": "nodemon"
+    "watch:server": "pm2 start ./test-server.ts --watch --no-daemon"


Worked great 👍

ajthinking · 2024-10-19T07:36:02Z

packages/ui/src/components/DataStory/DataStory.tsx

+    loading: diagramDataLoading,
+    error: diagramDataError
+  } = useRequest(async() => {
+    return client?.getDiagram?.({});


Same as before, on (?): can we require client always present?

ajthinking · 2024-10-19T07:37:12Z

packages/ui/src/components/DataStory/clients/WorkspaceApiClientBase.ts

@@ -21,15 +21,69 @@ export class WorkspaceApiClientBase implements WorkspaceApiClient {

  constructor(private transport: Transport) {
    this.initExecutionUpdateHandler();


Good shortenings below - keep same pattern here? ie initExecutionUpdates(); ?

ajthinking · 2024-10-19T07:37:57Z

packages/ui/src/components/DataStory/clients/WorkspaceApiClientBase.ts

    this.run = this.run.bind(this);
    this.updateDiagram = this.updateDiagram.bind(this);
  }

-  //<editor-fold desc="Message handler">


Seen these a few times - not sure when/how introduced?

This is a comment used for code folding, allowing us to keep the methods related to init folded.

expanded mode

collapsed mode

packages/ui/src/components/DataStory/clients/createTransport.ts

…m MockJSServer is synchronous

stone-lyl · 2024-10-21T08:52:08Z

Possible updateDiagram bug?

This is not a bug.
Since the socket server doesn't save the diagram, it doesn't implement the updateDiagram method.

Unfolding bug

I've resolved the issue, but I found another one : the eventManager is shared on the same page. When I run the Unfolded Main diagram demo, the Main diagram demo resets to its initial state.
I'm not quite sure if this issue needs to be addressed right away.

ajthinking · 2024-10-21T18:14:49Z

Possible updateDiagram bug?

This is not a bug. Since the socket server doesn't save the diagram, it doesn't implement the updateDiagram method.

Unfolding bug

I've resolved the issue, but I found another one : the eventManager is shared on the same page. When I run the Unfolded Main diagram demo, the Main diagram demo resets to its initial state. I'm not quite sure if this issue needs to be addressed right away.

I see!

Yes lets "solve" this for now by only supporting one component per page 👍

stone-lyl added 13 commits October 16, 2024 16:32

feat: split messageHandlers method and implement store diagram

d6a1bdd

feat: adjust types and implement getDiagram and updateDiagram methods

ad1b985

fix: resolve issue in the playground demo

9ae68a0

feat: add getDiagram method to WorkspaceApiClient

54557c7

fix: address issue with not receiving subscribed content before post …

b276917

…message

feat: add eventManager to updateDiagram method

da920f0

feat: add getDiagram in SocketServer

3d6f081

feat: replace onGetDirtyFileContent with getDiagram

1f39f7d

fix: resolve diagram type error in updateDiagram function

a27339d

feat: remove the fileUri variable

fe33a0a

feat: remove the "SAVE_SUCCESS" toast and save button

9d5e5af

fix: resolve node server coul not auto restart

b89d1d9

fix: resolve issue where diagram did not exist in localStorage

a9c0288

vercel bot deployed to Preview October 18, 2024 09:56 View deployment

fix: resolve the "playground not found" issue in e2e tests

d459720

vercel bot deployed to Preview October 18, 2024 10:24 View deployment

feat: Rename handle prefix to init in WorkspaceApiClientBase class

207a457

vercel bot deployed to Preview October 18, 2024 12:12 View deployment

stone-lyl requested a review from ajthinking October 18, 2024 14:31

ajthinking requested changes Oct 19, 2024

View reviewed changes

stone-lyl added 3 commits October 21, 2024 15:23

fix: need to call the listener handler synchronously, as the data fro…

2abb78a

…m MockJSServer is synchronous

feat: rename initExecutionUpdateHandler to initExecutionUpdates

37a42b2

feat: implement default value for getDiagram method in ds-ext

3afb229

vercel bot deployed to Preview October 21, 2024 08:26 View deployment

stone-lyl requested a review from ajthinking October 21, 2024 08:53

ajthinking approved these changes Oct 21, 2024

View reviewed changes

stone-lyl merged commit bf0f192 into ajthinking:main Oct 22, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: restore documentation examples #321

feat: restore documentation examples #321

stone-lyl commented Oct 18, 2024 •

edited

Loading

vercel bot commented Oct 18, 2024 •

edited

Loading

ajthinking left a comment

ajthinking Oct 19, 2024

ajthinking Oct 19, 2024

stone-lyl Oct 21, 2024 •

edited

Loading

ajthinking Oct 19, 2024

ajthinking Oct 19, 2024 •

edited

Loading

ajthinking Oct 19, 2024

ajthinking Oct 19, 2024

stone-lyl Oct 21, 2024

stone-lyl commented Oct 21, 2024

Possible updateDiagram bug?

Unfolding bug

ajthinking commented Oct 21, 2024

Possible updateDiagram bug?

Unfolding bug

		@@ -21,15 +21,69 @@ export class WorkspaceApiClientBase implements WorkspaceApiClient {

		constructor(private transport: Transport) {
		this.initExecutionUpdateHandler();

feat: restore documentation examples #321

feat: restore documentation examples #321

Conversation

stone-lyl commented Oct 18, 2024 • edited Loading

website playground:

node server playground:

vscode

vercel bot commented Oct 18, 2024 • edited Loading

ajthinking left a comment

Choose a reason for hiding this comment

Testing pm2

Possible updateDiagram bug?

Unfolding bug

Vscode broken

ajthinking Oct 19, 2024

Choose a reason for hiding this comment

ajthinking Oct 19, 2024

Choose a reason for hiding this comment

stone-lyl Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

ajthinking Oct 19, 2024

Choose a reason for hiding this comment

ajthinking Oct 19, 2024 • edited Loading

Choose a reason for hiding this comment

ajthinking Oct 19, 2024

Choose a reason for hiding this comment

ajthinking Oct 19, 2024

Choose a reason for hiding this comment

stone-lyl Oct 21, 2024

Choose a reason for hiding this comment

stone-lyl commented Oct 21, 2024

Possible updateDiagram bug?

Unfolding bug

ajthinking commented Oct 21, 2024

Possible updateDiagram bug?

Unfolding bug

stone-lyl commented Oct 18, 2024 •

edited

Loading

vercel bot commented Oct 18, 2024 •

edited

Loading

stone-lyl Oct 21, 2024 •

edited

Loading

ajthinking Oct 19, 2024 •

edited

Loading