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

Dependency on Giraffe feels excessive for the amount of HTML templating performed by the library #471

Open
nbevans opened this issue Sep 10, 2024 · 2 comments

Comments

@nbevans
Copy link

nbevans commented Sep 10, 2024

The Giraffe dependency should be reconsidered.

I understand it simplifies the HTML templating code - but really, the cost of the additional dependency is excessive, and there isn't that much HTML templating in the library anyway.

Unless there are plans to massively increase the amount of HTML templating in a future version of Plotly.NET I would suggest refactoring away from the Giraffe dependency.

Open to discussion

@kMutagene
Copy link
Collaborator

kMutagene commented Sep 10, 2024

The initial reason for the Giraffe.ViewEngine (not Giraffe!) dependency was twofold: 1. having type safe abstractions for the templating resolved a lot of issues regarding typos etc. (initially, we did simple string interpolation), and 2. providing Giraffe-compatible html node output that can be used in any giraffe server application.

The second reason fell flat as soon as we ran into all the bullshit issues with strong naming this assembly (see #464), but i have nonetheless kept it since. F# has a general problem that there are multiple html abstractions out there and i wanted to prevent having a library containing another implementation of that (also, i do not want to maintain a html template lib/module really).

So once we get rid of the strong name, we get back the compatibility with giraffe applications and imo are fine. Also, i do not really get what the 'cost of a dependency being excessive' means here - Giraffe.ViewEngine has 0 dependencies and is 126.46 KB - could you elaborate on that?

Now that i think about it though - we could separate the html templating from the core lib completely, by also publishing Plotly.NET.HTML as its own project/nuget package similarly to what we do with Plotly.NET.ImageExport. That way, consuming code that really does not want that dependency could implement its own html conversion for the GenericChart type. Sounds a bit over-engineered though.

@nbevans
Copy link
Author

nbevans commented Sep 11, 2024

It's not that the Giraffe.ViewEngine is big or complex (it isn't) - it's just another dependency that needs pulling in and its lifecycle will need to be managed by yourselves and Plotly.NET users. I checked how much code in Plotly.NET is actually using this library and it isn't very much (yet). In my view, the previous way you were doing it via "[FOO]" style placeholder string replacements was superior because, although many people might call it hacky, it was dead simple, easy to reason about and didn't require this additional dependency.

I don't know what license the Giraffe ViewEngine is, assuming its compatible, have you considered just pulling in the source code directly into Plotly.NET so that you can manage the lifecycle of it yourselves and avoid the dependency?

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

No branches or pull requests

2 participants