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

feat: Add HTML minification for rendering #2437

Closed

Conversation

alwismt
Copy link

@alwismt alwismt commented Apr 26, 2023

The function has been added to minify the HTML data into a token stream, and then write the minified HTML to a buffer. This feature improves performance by reducing the size of the HTML output.

Description

Added minify.go file with a htmlMinify function that can be used to minify HTML data. Also wrote a test case TestHtmlMinify in minify_test.go to ensure it works as expected. Added the htmlMinify function to Ctx.Render function in Ctx.go and also added an EnableHTMLMinify app config option for it. Wrote another test function Test_Ctx_RenderWithMinify in Ctx_test.go to test rendering with minification enabled.

Type of change

New feature

Checklist

  • I have added test cases to ensure my changes work as expected
  • I have updated the documentation accordingly
  • My code follows the style guidelines of the project
  • I have checked that there are no linting errors in my code

Commit formatting

feat: added minify function for HTML rendering and added tests

Changes

  • Added minify.go file with htmlMinify function
  • Added TestHtmlMinify test case in minify_test.go
  • Added htmlMinify function to Ctx.Render in Ctx.go
  • Added EnableHTMLMinify app config option for minification
  • Added Test_Ctx_RenderWithMinify test function in Ctx_test.go

@welcome
Copy link

welcome bot commented Apr 26, 2023

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

Copy link
Member

@ReneWerner87 ReneWerner87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting and good feature,
Thanks for the work

Can we modify it a bit, currently the feature only looks at the render method, but could be applied to all responses which matched certain content types

I have a middleware in mind that you can add like the compress middleware and then you can configure it to enable different minification on the different routes.

Maybe we should also provide this in the contrib repository, as I don't want to bloat the core (if it's a middleware, we can vote and move the code)

@alwismt
Copy link
Author

alwismt commented Apr 26, 2023

Great, sounds like a good plan. I'll work on implementing this feature in the compress middleware so that it can minify other content types beside HTML.

Additionally, I can create a separate middleware for minification, which can be configured to enable different minification on different routes.

Regarding the contrib repository, that's a great idea as well. We can move the code to the contrib repository if it's agreed upon by the community. Thank you for your feedback and suggestions.

@ReneWerner87
Copy link
Member

Great, sounds like a good plan. I'll work on implementing this feature in the compress middleware so that it can minify other content types beside HTML.

here you misunderstood something, i meant the compress middleware you can use as a template
the actual compress middleware should not be adapted of course

@alwismt
Copy link
Author

alwismt commented Apr 26, 2023

Got it, I will use the compress middleware as a reference and will create a new middleware for minification of different content types.

@alwismt alwismt closed this Apr 26, 2023
@alwismt alwismt force-pushed the feature/add-html-minification-for-render branch from 3ebfd28 to 0b5baf5 Compare April 26, 2023 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants