go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
44.16k stars 5.41k forks source link

Mermaid.js default character limit is too low #32015

Open neon-dev opened 1 week ago

neon-dev commented 1 week ago

Description

Mermaid.js has a default maxTextSize of 50000: https://github.com/mermaid-js/mermaid/blob/dd0304387e85fc57a9ebb666f89ef788c012c2c5/packages/mermaid/src/schemas/config.schema.yaml#L95-L98

Can we increase https://github.com/go-gitea/gitea/blob/a323a82ec4bde6ae39b97200439829bf67c0d31e/modules/setting/markup.go#L64 to the same value?

Gitea Version

1.22.2

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

-

Database

None

yp05327 commented 1 week ago

But this is configurable.

neon-dev commented 1 week ago

Is there a reason to keep the default value different from what the creators of mermaid.js intended?

yp05327 commented 1 week ago

I don't know why, but I found this: https://github.com/go-gitea/gitea/issues/16513#issuecomment-885006410 Maybe 5000 is older than this default value.

neon-dev commented 1 week ago

Looks like there was no limit a few years ago: https://github.com/mermaid-js/mermaid/commit/4ad354a561a941d968ebf8b91edf0309c0fe0d03 Maybe the config value in Gitea should become optional then?

neon-dev commented 5 days ago

I also checked out GitLab's original implementation of the character limit that was mentioned in your linked comment. It is just supposed to defer rendering of larger graphs so it doesn't slow down the initial page load. There is no hard limit that denies rendering them altogether like Gitea currently handles this. And from my limited testing on GitHub, they don't even seem to have any limit or it is not triggered easily.

In summary, Gitea's current implementation and default value try to solve one issue by creating an entirely new one: Not being able to render a graph. I suggest evaluating at what threshold performance issues start to ocurr (should only happen with multiple graphs on a page) and deciding a new strategy to handle such cases. GitLab also revised their strategy with a later commit by tracking how many elements have been rendered in total on a page.