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

Replace Mermaid #102

Open
keonik opened this issue May 27, 2022 · 12 comments
Open

Replace Mermaid #102

keonik opened this issue May 27, 2022 · 12 comments

Comments

@keonik
Copy link
Owner

keonik commented May 27, 2022

Most of the issues related to this repository have been related to getting mermaid cli to generate a file. Variables that I've noticed so far are

There are also ideas for extension such as

  • Large schema(s) not filling space in a smart way
  • Limited color schemes

This issue is to discuss what we would need out of a replacement. If there is enough interest maybe we could get this started

@keonik
Copy link
Owner Author

keonik commented Jun 8, 2022

Also #104

@icazemier
Copy link

We've seen a considerable amount of disk space occupied bij the installed npm package of Mermaid cli.
Confusing is they expect to have puppeteer installed also (why isn't this in mentioned in their .npmignore)?

[Our project]
└─┬ @mermaid-js/mermaid-cli@9.1.2
  └── puppeteer@14.1.1

ivo@Ivos-MBP vecore-database-schema % 

A scan by GrandPerspective:
image

@cjoecker
Copy link

I did the first try using Graphviz instead of mermaid and this is the result:
image

The lines don't look as clear as with mermaid but in my opinion, it looks nicer now.
The only thing is that the relationship labels may sometimes look cluttered next to some primary keys like in the User table.
I will continue working on it and submit a PR.

@keonik
Copy link
Owner Author

keonik commented Aug 29, 2022

@cjoecker Share a PR whenever you have it in a good spot. I think it looks way better already. Would be interested to see how it handles the huge schema's in the test cases in the repo.

@keonik
Copy link
Owner Author

keonik commented Sep 8, 2022

#129 special character limitation. Overlap with a regex problem but when I fix it it will cause failures at the mermaid mmdc step

@SkeLLLa
Copy link

SkeLLLa commented Sep 11, 2022

Mermiad has a big advantage, generated diagrams could be embedded to markdown files and github, for example, could render them. So I think it worth not to "replace" but maybe add some alternative.

Maybe it could be done by creating some kinda plugins. The main module will be responsible for parsing prisma schema and returning some structure that could be later transform to graph.
And mermaid, graphvis or any other visualization plugin will be responsible for visualizing structure from main module.

@cjoecker
Copy link

@keonik I created a draft PR with the proof of concept for Graphviz. Let me know what you think so that I can start the refactoring, tests fix etc.

@SkeLLLa you have a good point about the embedded diagrams in the markdown files I haven't thought about. Diagrams can also be embedded and rendered as SVG files. So, whenever the schema is generated, the old SVG will be replaced and the markdown files are updated automatically 👏🏻

@cjoecker
Copy link

@keonik did you see the pull request? What do you think?

@keonik
Copy link
Owner Author

keonik commented Nov 28, 2022

@keonik did you see the pull request? What do you think?

I gave it a shot running locally and had a few TypeScript errors and missing imports that hung up a successful test. It says its missing generate_old.ts which has some of the DML types. I'm happy to support Graphviz but some people are big fans of mermaid still. Makes me think providing a configuration option to pick between Graphviz and Mermaid is in order. One of the big complaints against mermaid is that we need to run a cli to generate the file if it isn't in markdown format. This is still in need of running a cli command so it doesn't rule out one of the common OS and versioning issues people are currently running into.

@cjoecker
Copy link

cjoecker commented Dec 8, 2022

@keonik I just pushed the missing file.

It is also ok for me to have mermaid and graphviz together. This PR was just a try to see if it works. I can now do a cleaner for that.

For implementing a final solution, It would be great if we have a function that gives us back the data in the following structure:

export interface TemplateValues {
    tables: Table[];
    relationships: Relationship[];
}

export interface Table {
    name: string;
    fields: {
        name: string;
        primaryKey: string;
    }[];
}

export interface Relationship {
    fromTableName: string;
    fromFieldIndex: number;
    fromRelationshipType: '1' | '*';
    toTableName: string;
    toFieldIndex: number;
    toRelationshipType: '1' | '*';
}

Then it would be easier to use different engines to generate the graphic.
What do you think about joining efforts for that? You can refactor the library to make it more modular and deliver that data structure. Then, I can implement the Graphviz generation.

@keonik
Copy link
Owner Author

keonik commented Mar 21, 2023

@cjoecker What are your thoughts on having separate branches for these representations 🤔

I just recently setup the ability to push to alpha and bravo branches that create future implementations for testing ease. I could just as easily add a graphviz branch and you would just change the install to something like this

npm i -D prisma-erd-generator@graphviz

@cjoecker
Copy link

@keonik separate branches are a good alternative. I just ask myself if it wouldn't be better to add it just as a prop in the configuration of the plugin. It is more user-friendly in my opinion. Also, if the objects and logic to produce the diagrams is clean as mentioned here, it would be super easy to use different rendering engines based on the settings props. But I struggled a bit to transform the current logic to graphviz because it was a lot of strings concatenation that was difficult to understand for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants