vizdom-dev / vizdom

A TypeScript library for Vizdom
Apache License 2.0
135 stars 1 forks source link

Feedback #9

Open stereobooster opened 2 days ago

stereobooster commented 2 days ago

Hi 👋

Some feedback (feel free to discard):

s0l0ist commented 2 days ago

Hi @stereobooster, thanks for your feedback!

s0l0ist commented 2 days ago

I've confirmed the library is working, but you need the esm package.

I used the following:

Modified the vite.config.js file:

import react from "@vitejs/plugin-react-swc";
import { defineConfig } from "vite";
import wasm from "vite-plugin-wasm";

// https://vitejs.dev/config/
export default defineConfig({
  build: {
    ssr: true,
  },
  plugins: [react(), wasm()],
});

Here's the App.tsx file:

import { DirectedGraph } from '@vizdom/vizdom-ts-esm'
import { useState } from 'react'
import './App.css'
import reactLogo from './assets/react.svg'
import viteLogo from '/vite.svg'

function App() {
  const [count, setCount] = useState(0);
  const [svgString, setSvgString] = useState("");
  const graph = new DirectedGraph();

  const v0 = graph.new_vertex({
    render: {
      label: "origin"
    }
  })

  const updateGraph = () => {
    const positioned = graph.layout();
    const svg = positioned.to_svg();
    const svgString = svg.to_string();
    setSvgString(svgString);
  };

  const handleButtonClick = () => {
    const newVertex = graph.new_vertex({
      render: {
        label: `${count}`
      }
    });
    graph.new_edge(v0, newVertex, {
      render: {
        label: `edge to ${count}`
      }
    });
    setCount(count + 1);
    updateGraph();
  };

  return (
    <>
      <div>
        <a href="https://vitejs.dev" target="_blank">
          <img src={viteLogo} className="logo" alt="Vite logo" />
        </a>
        <a href="https://react.dev" target="_blank">
          <img src={reactLogo} className="logo react" alt="React logo" />
        </a>
      </div>
      <h1>Vite + React</h1>
      <div className="card">
        <div dangerouslySetInnerHTML={{ __html: svgString }} />
        <button onClick={handleButtonClick}>
          count is {count}
        </button>
        <p>
          Edit <code>src/App.tsx</code> and save to test HMR
        </p>
      </div>
      <p className="read-the-docs">
        Click on the Vite and React logos to learn more
      </p>
    </>
  )
}

export default App

Here's my basic package.json:

{
  "name": "vite-project-2",
  "private": true,
  "version": "0.0.0",
  "type": "module",
  "scripts": {
    "dev": "vite",
    "build": "tsc -b && vite build",
    "lint": "eslint .",
    "preview": "vite preview"
  },
  "dependencies": {
    "@vizdom/vizdom-ts-esm": "^0.1.9",
    "react": "^18.3.1",
    "react-dom": "^18.3.1"
  },
  "devDependencies": {
    "@eslint/js": "^9.11.1",
    "@types/react": "^18.3.10",
    "@types/react-dom": "^18.3.0",
    "@vitejs/plugin-react-swc": "^3.5.0",
    "eslint": "^9.11.1",
    "eslint-plugin-react-hooks": "^5.1.0-rc.0",
    "eslint-plugin-react-refresh": "^0.4.12",
    "globals": "^15.9.0",
    "typescript": "^5.5.3",
    "typescript-eslint": "^8.7.0",
    "vite": "^5.4.8",
    "vite-plugin-wasm": "^3.3.0"
  }
}
stereobooster commented 1 day ago

I tried vite-plugin-wasm, the problem is that it may work with Vite, but not with Vitest and not with Astro. Similar issue reported here https://github.com/Menci/vite-plugin-wasm/issues/56.

Which is ok with me - I got my workaround. I just wanted let you know

s0l0ist commented 1 day ago

Now, going to check on Vitest, Astro

s0l0ist commented 1 day ago

Okay, I have an example Vitest + Astro, but you need to use the node distribution in the tests. There's some issue with the way it works with the esm version of the library as you pointed out. A solution is to mock the import for @vizdom/vizdom-ts-esm and replace with @vizdom/vizdom-ts-node.

basic.test.ts:

import { test, vi } from "vitest";

// Mock the ESM version to use the Node.js version in tests
vi.mock("@vizdom/vizdom-ts-esm", async () => {
  return await import("@vizdom/vizdom-ts-node");
});

// In some module, where it uses the `esm` library, the mock should override
// with the node version.
import { DirectedGraph } from "@vizdom/vizdom-ts-esm";

// Edit an assertion and save to see HMR in action
test("Vizdom", () => {
  let graph = new DirectedGraph();
  let v0 = graph.new_vertex({
    render: {
      label: "a",
    },
  });
  let v1 = graph.new_vertex({
    render: {
      label: "b",
    },
  });
  let e0 = graph.new_edge(v0, v1, {
    render: {
      label: `${v0.to_json().render.label} --> ${v1.to_json().render.label}`,
    },
  });
  let positioned = graph.layout();
  let json = positioned.to_json();
  let json_str = json.to_string_pretty();
  console.log(json_str);
});
stereobooster commented 1 day ago

There maybe a way to dynamically load a version of the library depending on the runtime, but I'm not aware of a quick solution.

exports field in package json maybe? I think one can put browser option there, so it would be used for browsers only.

This is general problem. A lot of packages convert binary to Base64 (or Base92, and compress it, and use something like https://github.com/101arrowz/fflate to decompress) and inline string. Which is horrible way to do it, but it is the most portable way.

Anyway. I didn't know about @vizdom/vizdom-ts-node. I think it will solve my problem 👍

s0l0ist commented 1 day ago

Ah yeah I know exactly what you're talking about with base64ing the WASM and load as a string. I actually tried hard NOT to do it here in this library to not bloat it so much. I had to do it in another project and thought maybe the ecosystem handles WASM better than 4 years ago - which it certainly does, but still not super well.

BTW here's a test example:

// ./src/viz.ts
import { DirectedGraph } from "@vizdom/vizdom-ts-esm";

export function createGraph() {
  const graph = new DirectedGraph();

  const v0 = graph.new_vertex({
    render: {
      label: "a",
    },
  });
  const v1 = graph.new_vertex({
    render: {
      label: "b",
    },
  });

  graph.new_edge(v0, v1, {
    render: {
      label: `${v0.to_json().render.label} --> ${v1.to_json().render.label}`,
    },
  });

  const positioned = graph.layout();
  return positioned.to_json().to_string_pretty();
}

Test file:

// ./test/basic.test.ts
import { test, vi } from "vitest";

// Mock the ESM version to use the Node.js version in tests
vi.mock("@vizdom/vizdom-ts-esm", async () => {
  return await import("@vizdom/vizdom-ts-node");
});

// In some module, where it uses the `esm` library, the mock should override
// with the node version.
import { createGraph } from "../src/viz";

// Edit an assertion and save to see HMR in action
test("Vizdom", () => {
  let graph = createGraph();
  console.log(graph);
  // do real assertions :) 
});
s0l0ist commented 1 day ago

@stereobooster, I noticed in your plugin that you're removing the width and height. I can add some methods to optionally add them if needed so you don't have to 'hack' them out.

s0l0ist commented 1 day ago

You can now install the latest version (v0.1.12) and no longer need to remove the width and height attributes.

If you (or anyone) needs those attributes, you can simply call the appropriate method:

const positioned = graph.layout();
const svg = positioned.to_svg().with_width_and_height();
...
stereobooster commented 1 day ago

I noticed in your plugin that you're removing the width and height.

motivation behind this: when SVG is in-lined, allow to set width and height with CSS, so it would fit container automatically. Something like this:

width: 100%;
height: auto;

But it looks weird when graph is very small. I probably need to put some threshold (500px?) before I remove those attributes. This part needs more thought

stereobooster commented 1 day ago

A bit more feedback:

s0l0ist commented 1 day ago
s0l0ist commented 48 minutes ago

@stereobooster,

In the latest release these are addressed!

For DOT support:

digraph {
    URL="https://example1.com"
    a [URL="https://example2.com"]
    a -> b [URL="https://example3.com"]
}

The programmatic API also supports the ability to add the url parameter as a render attribute:

const graph = new DirectedGraph({
  render: {
    url: "https://vizdom.dev",
  }
})
// ...
const v0 = graph.new_vertex({
  render: {
    url: "https://vizdom.dev",
  }
})
// ...
const e0 = graph.new_edge(v0, v1, {
  render: {
    url: "https://vizdom.dev",
  }
})