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

Change default ERB template engine to erubi #1475

Closed
wants to merge 1 commit into from

Conversation

tkmru
Copy link
Contributor

@tkmru tkmru commented Sep 20, 2018

Erubi is a simplified fork of Erubis, Erubi is faster than erubis and erb.

In rails, default ERB template engine is erubi from rails 5.1(Change ActionView ERB Handler from Erubis to Erubi by jeremyevans · Pull Request #27757 · rails/rails).

Erubi offers the following advantages for erubis:

  • Handles postfix conditionals when using escaping (e.g. <%= foo if bar %>)
  • Supports frozen_string_literal: true in templates via :freeze option
  • Works with ruby's –enable-frozen-string-literal option
  • Automatically freezes strings for template text when ruby optimizes it (on ruby 2.1+)
  • Escapes ' (apostrophe) when escaping for better XSS protection
  • Has 6x faster escaping on ruby 2.3+ by using cgi/escape
  • Has 86% smaller memory footprint
  • Does no monkey patching (Erubis adds a method to Kernel)
  • Uses an immutable design (all options passed to the constructor, which returns a frozen object)
  • Has simpler internals (1 file, <150 lines of code)
  • Is not dead (Erubis hasn't been updated since 2011)

@tkmru tkmru changed the title change default ERB template engine to erubi Change default ERB template engine to erubi Sep 20, 2018
@tkmru
Copy link
Contributor Author

tkmru commented Oct 23, 2018

I'm sorry to be leaving that it fell in a test.

I'm confused at a result of the test. falls back to engine layout test was failed with Errno::ENOENT: No such file or directory @ rb_sysopen - sinatra/test/views/layout.erubi.
Why is this extension(.erubi) attached? .erubi does not exist in the code.

Copy link
Member

@namusyaka namusyaka left a comment

Choose a reason for hiding this comment

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

First of all, we don't need to set erubi in the erb helper because we're using Tilt. It handles the template engines switching automatically and it supports for erubi of course.
For example, if you've installed erubis on your app, your app will use erubis as a ERB template engine.
I would say the same is true of erubi.
If you'd like to use erubi on your app, you should just install erubi on your app.

Thus, your changes is not necessary for enabling erubi.
If you would like to continue the contribution around erubi, you can commit to the docs, testing, and the sinatra-contrib library.
Otherwise, please close this PR.

@@ -679,7 +679,8 @@ def initialize
end

def erb(template, options = {}, locals = {}, &block)
render(:erb, template, options, locals, &block)
require 'erubi'
Copy link
Member

Choose a reason for hiding this comment

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

Why do you add require 'erubi' in this? By reflecting this code, the method call for erb will largely be slowed down because the unnecessary method call is always happened.

@namusyaka
Copy link
Member

In addition to my comment,
we don't have the erubis as a default template engine. It's only enabled when erubis is required.

@tkmru
Copy link
Contributor Author

tkmru commented Oct 27, 2018

OK, Thank you for the comment.

@tkmru tkmru closed this Oct 27, 2018
@namusyaka namusyaka mentioned this pull request Oct 28, 2018
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

Successfully merging this pull request may close these issues.

None yet

2 participants