Bug/Feature: Partials not rendered correctly

[schneems]

Original issue: #53 (comment)

Copying because it's an old one and we have a new reproduction. If someone fixes this in October I'll hacktober-accepted label the PR.

Reproduction:

---

From @joshuapinter

I finally got around to creating a simple example app that re-creates the issue of markdown (.md.erb) partials not rendering correctly in plain text. You can view the repo here:

https://github.com/joshuapinter/maildown_partials_issue

I've included the repro instructions in the README but here they are as well:

1. Clone the repo and install gems via bundle.

2. Run the server via bundle exec rails s

3. Navigate to http://localhost:3000/rails/mailers/notifier_mailer/markdown_partial_test in your browser:

   

   By default the HTML format will appear, and it appears correctly.

4. Click on the format drop down and select "View as plain-text email".

   

   You will see that the partial gets rendered incorrectly as HTML instead of plain text.

5. To see what it should look like, navigate to http://localhost:3000/rails/mailers/notifier_mailer/non_markdown_partial_test.txt in your browser.

   

   You will see that using two files for formats (html.erb and text.erb) and not using markdown (md.erb) via maildown renders the partials correctly for both HTML and plain text.

I did this in Rails 6 and Ruby 3 because that's what I had handy.

I hope this is enough to re-open this Issue and take another look. I've spent a few hours trying to figure it out myself and digging into how Rails renders partials but it's a bit over my head.

Let me know if you need anything from my side or if you want me to do some testing.

Rendering emails in markdown is incredible and I feel like this is the missing piece to make it truly clean, DRY and beautiful.

---

My analysis

How maildown works: It first renders ERB and then runs it through a Markdown parser to generate HTML, or just plain text to deliver the text/plain version. What I think is happening here is that the ERB in the partial is being evaluated as if it's an HTML partial no matter what.

I think there's two paths forward:

• Find out where the mime type for the partial is being set and either monkeypatch to hook into Rails and set it, or through some other more durable mechanism.
• If that doesn't work, the fallback question could be "can we determine which view is being rendered from within the view?" If so, then maybe the partial rendering method in Rails has some way to pass in extra content
• If all of that fails, maybe there's still some way to workaround the problem that investigation might show. If that happens documenting it as a known workaround could be helpful.

[schneems]

I poked at this for awhile. The problem is that the format isn't being persisted through the render call to the partial. Each template has a format method that should be :html or :text` and in this case the partial is rendered with a nil format.

I don't know where or how that value is supposed to be set :(

It might be possible to reproduce this without maildown. I think that's a good step, but I don't know of any other template that changes behavior based on format. Basically: I think this is a Rails bug, but I can't prove it's not caused by a maildown monkeypatch.

We can access the existing templates formats array but I'm not sure if there's a good interface for passing that to render or if it even will work that way. In general the render method is a beast so it could be possible but it's really hard to know for sure.

Meta thoughts

The maildown project is hard to understand here's some intermediate steps:

• We need to remove support for Rails < 6 and reduce/remove monkeypatches aggressively. Document what each piece does and does not do. Maybe make some kind of architectural diagram or something.
• Open issues with rails/rails for each of the monkey-patches we need. It should be better to use a supported interface. I ge the feeling this will be a "PRs welcome" sort of issue but we could get lucky.
• Barring that: Consider non-monkey-patch alternatives. I had an idea for defining a markdown template and then having a rake task that turned it into an HTML template on disk.

[joshuapinter]

@schneems Thanks so much for looking into this. Seems I'm not the only one that found the render function to be a monolithic beast!

If I recall, my original investigation found something similar, that the format gets lost when rendering partials.

This may be a Rails bug itself but my thinking that it wasn't and it should be fixable is that when rendering partials without maildown, the format gets properly passed and the correct partial format is used (e.g. html.erb or text.erb).

But I know there's more to it with maildown.

Again, I honestly think this is the final feature to round out the functionality of maildown.

I'm not sure what else I can do here but let me know if there's anything. Would love to get this in there and remove the hacks I've had to use to get around this.

Thanks again! 🙏

[schneems]

I'm working on cleaning up the project and removing Rails 5 support #70. Github actions are failing when setting up Ruby versions ATM, no clue why :(

I think I could reproduce the problem without any monkeypatches. Rails 7 only uses one monkeypatch and it's mostly for convenience to avoid having to call format.text and format.html in every mailer.

Honestly, this project isn't doing all that much once the branching conditionals for Rails < 5 are removed. It's mostly just using APIs provided via Rails.

[joshuapinter]

Well, maybe now's the time to do a v4 branch and only support Rails 7+ going forward and include this proper format handling on partials.

We're running the latest version of Ruby and Rails if you want some testing done. Keen to help anyway we can and can afford with time allowances, etc.

Thanks and Happy New Year!