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

Rails 7 1 support #71

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

cowlibob
Copy link

Rails 7.1 appears to have changed what is passed into the MailDown::MarkdownEngine#to_html. What used to be a String (or String-like object) is now an ActionView::OutputBuffer.

ActionView::OutputBuffer does not respond to encode, valid_encoding? and other String methods called inside Kramdown.

The simplest fix I could muster is to convert to a String before use. Do feel free to consider other solutions though, if you feel it appropriate.

Using the gemfiles/rails_7_1_gemfile and it's lock without the fix (commit b520fae) will show the following (truncated) output during tests:

Error:
LayoutsTest#test_layout_renders_fine:
ActionView::Template::Error: undefined method `valid_encoding?' for an instance of ActionView::OutputBuffer
    kramdown (2.4.0) lib/kramdown/parser/base.rb:92:in `adapt_source'
    kramdown (2.4.0) lib/kramdown/parser/kramdown.rb:90:in `parse'
    kramdown-parser-gfm (1.1.0) lib/kramdown/parser/gfm.rb:56:in `parse'
    kramdown (2.4.0) lib/kramdown/parser/base.rb:69:in `parse'
    kramdown (2.4.0) lib/kramdown/document.rb:102:in `initialize'
    /Users/james/Projects/James/maildown/lib/maildown/markdown_engine.rb:53:in `new'
    /Users/james/Projects/James/maildown/lib/maildown/markdown_engine.rb:53:in `block in default_html_block'
    /Users/james/Projects/James/maildown/lib/maildown/markdown_engine.rb:24:in `to_html'
    app/views/user_with_layout_mailer/welcome.md+erb:1
    actionview (7.1.2) lib/action_view/base.rb:264:in `public_send'
    actionview (7.1.2) lib/action_view/base.rb:264:in `_run'
    actionview (7.1.2) lib/action_view/template.rb:261:in `block in render'
LayoutsTest#test_no_layout:
ActionView::Template::Error: undefined method `valid_encoding?' for an instance of ActionView::OutputBuffer
    kramdown (2.4.0) lib/kramdown/parser/base.rb:92:in `adapt_source'
    kramdown (2.4.0) lib/kramdown/parser/kramdown.rb:90:in `parse'
    kramdown-parser-gfm (1.1.0) lib/kramdown/parser/gfm.rb:56:in `parse'
    kramdown (2.4.0) lib/kramdown/parser/base.rb:69:in `parse'
    kramdown (2.4.0) lib/kramdown/document.rb:102:in `initialize'
    /Users/james/Projects/James/maildown/lib/maildown/markdown_engine.rb:53:in `new'
    /Users/james/Projects/James/maildown/lib/maildown/markdown_engine.rb:53:in `block in default_html_block'
    /Users/james/Projects/James/maildown/lib/maildown/markdown_engine.rb:24:in `to_html'
    app/views/user_no_layout_mailer/welcome.md+erb:1
    actionview (7.1.2) lib/action_view/base.rb:264:in `public_send'
    actionview (7.1.2) lib/action_view/base.rb:264:in `_run'
    actionview (7.1.2) lib/action_view/template.rb:261:in `block in render'

Running the same test suit on (commit 2347d47) will pass.

See tests fail with following stack:

LayoutsTest#test_no_layout:
ActionView::Template::Error: undefined method `valid_encoding?' for an instance of ActionView::OutputBuffer
    kramdown (2.4.0) lib/kramdown/parser/base.rb:92:in `adapt_source'
    kramdown (2.4.0) lib/kramdown/parser/kramdown.rb:90:in `parse'
    kramdown-parser-gfm (1.1.0) lib/kramdown/parser/gfm.rb:56:in `parse'
    kramdown (2.4.0) lib/kramdown/parser/base.rb:69:in `parse'
    kramdown (2.4.0) lib/kramdown/document.rb:102:in `initialize'
    /Users/james/Projects/James/maildown/lib/maildown/markdown_engine.rb:53:in `new'
    /Users/james/Projects/James/maildown/lib/maildown/markdown_engine.rb:53:in `block in default_html_block'
    /Users/james/Projects/James/maildown/lib/maildown/markdown_engine.rb:24:in `to_html'
    app/views/user_no_layout_mailer/welcome.md+erb:1
    actionview (7.1.2) lib/action_view/base.rb:264:in `public_send'
    actionview (7.1.2) lib/action_view/base.rb:264:in `_run'
    actionview (7.1.2) lib/action_view/template.rb:261:in `block in render'
…string. Several String methods are used, but unsupported on the ActionView::OutputBuffer (for example `valid_encoding?` and `encode`).
@joshuapinter
Copy link

Hey @schneems, any chance we can get this looked and merged in, if good. We're in the process of upgrading to Rails 7.1 and this is a super useful gem we use to DRY up a tonne of our views.

Let me know what I can do to help too.

Thanks!

joshuapinter added a commit to cntral/maildown that referenced this pull request Jan 15, 2024
Handle when `string` is a `ActionView::OutputBuffer` by converting it to a `String` first.

Pulled from zombocom#71.
joshuapinter added a commit to cntral/maildown that referenced this pull request Jan 15, 2024
Handle when `string` is a `ActionView::OutputBuffer` by converting it to a `String` first.

Pulled from zombocom#71.
joshuapinter added a commit to cntral/maildown that referenced this pull request Jan 22, 2024
Handle when `string` is a `ActionView::OutputBuffer` by converting it to a `String` first.

Pulled from zombocom#71.
vietqhoang added a commit to tofugu/maildown that referenced this pull request Jan 22, 2024
Adds and addresses the following:

* Adds Rails 7.1 to test matrix
* Addresses expected String type when rendering (zombocom#71)
@kieranklaassen
Copy link

would love this merged!

@roelbondoc
Copy link

I too would like to see this merged in.

@cowlibob If I may, there is one suggestion I'd like to make for this PR. I think we just need to ensure only strings are passed into kramdown. So an alternative to modifying the self.to_html and self.to_text method, is to modify the self.default_html_block method instead. Here's what it looks like in my fork.

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.

4 participants