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

Stop building CSS and adopt github-markdown-css instead #55

Open
kidonng opened this issue Mar 30, 2023 · 6 comments
Open

Stop building CSS and adopt github-markdown-css instead #55

kidonng opened this issue Mar 30, 2023 · 6 comments

Comments

@kidonng
Copy link
Contributor

kidonng commented Mar 30, 2023

github-markdown-css was initially suggested as an attempt to reduce bundle size in #2, but at that time I feel this could result in being neglected.

Now that this project is under denoland umbrella and getting proper maintenance, it makes sense to ask: Is bundling CSS necessary? Won't that be too opinionated?

I think it would better to focus on the actual rendering, and dedicate styling to people who are doing it seriously in a standalone object.

This also covers the recently added Katex styles, which is a opt-in feature to begin with.

It may look inconvenient as one has to fetch the style themselves, but I consider this a push for the good: you should use <link rel="stylesheet"> instead of inlining this many styles after all.

@lino-levan
Copy link
Contributor

I'm -1 on not shipping any CSS at all. People who care about the difference between inlining styles and linking a stylesheet already do what makes sense for them (ref: Pyro). People who want to use different CSS can if they want to (we should have better docs on how to do that). I definitely think that our current implementation of bundling CSS is suboptimal and could be improved.

and dedicate styling to people who are doing it seriously in a standalone object.

Are you referring to github-markdown-css or another project here? Is there more than one project doing this?

@kidonng
Copy link
Contributor Author

kidonng commented Mar 30, 2023

People who care about the difference between inlining styles and linking a stylesheet already do what makes sense for them

When the docs say nothing about it, people would not care. Find a real example where:

  • CSS is served as an actual file
  • <link rel="stylesheet"> is used to request that file

I definitely think that our current implementation of bundling CSS is suboptimal and could be improved.

Then do not do it at all. This project is initially just for simple markdown rendering for deno.land, the choice to host styles in house only made sense at that time.

Are you referring to github-markdown-css or another project here? Is there more than one project doing this?

As far as I know github-markdown-css is the only one doing similar things. I said people because, well, it's not just one person working on that.

@lino-levan
Copy link
Contributor

Find a real example where...

Pyro does that, check out the head of the website.

Then do not do it at all.

This seems dubious to me. I call it suboptimal because I rewrote the old implementation to what we have today and know that the build system is pretty fragile. In my eyes, we would ideally switch to github-markdown-css instead of trying to do the generation ourselves. I think we should write docs (in general) and tell people who care that they can use a linked stylesheet.

This project is initially just for simple markdown rendering for deno.land, the choice to host styles in house only made sense at that time.

I think it still has some merit in its simplicity today. That's the big selling point of deno-gfm to me.

I said people because, well, it's not just one person working on that.

Thanks for clarifying, 👍

@kidonng
Copy link
Contributor Author

kidonng commented Mar 30, 2023

Pyro does that

I see where you are coming from, but many people are not seasoned enough to make the same choice.

In my eyes, we would ideally switch to github-markdown-css instead of trying to do the generation ourselves.

I probably should have clarified, that is the real thing I want to get rid of. I'm not saying the CSS export should be nuked entirely (which would very unlikely to be accepted by the maintainers), it could be changed to re-export github-markdown-css.

I think we should write docs (in general) and tell people who care that they can use a linked stylesheet.

I would certainly resort to better docs if changing actual code is not approved.

I think it still has some merit in its simplicity today. That's the big selling point of deno-gfm to me.

Same for me. I was just stating why does this package bundles styles to begin with.

@lino-levan
Copy link
Contributor

I probably should have clarified, that is the real thing I want to get rid of. I'm not saying the CSS export should be nuked entirely (which would very unlikely to be accepted by the maintainers), it could be changed to re-export github-markdown-css.

Ah sorry, I misinterpreted your message. I completely agree with this sentiment. I don't see a reason why a code change would not be accepted especially since it greatly simplifies our fragile workflow.

@kidonng kidonng changed the title Stop shipping CSS and suggest github-markdown-css instead Stop building CSS and adopt github-markdown-css instead Mar 30, 2023
@kidonng
Copy link
Contributor Author

kidonng commented Mar 30, 2023

I don't see a reason why a code change would not be accepted especially since it greatly simplifies our fragile workflow.

I was referring to what I initially suggested: removing CSS from this package completely, which could be deemed too aggressive and breaking for little good.

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