Skip to content

Add Rails 6 Support#15

Open
arlando wants to merge 3 commits intobillhorsman:masterfrom
arlando:rails6-support
Open

Add Rails 6 Support#15
arlando wants to merge 3 commits intobillhorsman:masterfrom
arlando:rails6-support

Conversation

@arlando
Copy link
Copy Markdown

@arlando arlando commented May 15, 2020

Hi @billhorsman

I am working on having this library support Rails 6. I ran into a few regressions with the inline-styles-mailer gem against Rails 6. I've tried to add support while maintaining backwards compatibility as much as possible.

In Rails 6 the _layout method's arity has changed. The render_to_string method when used with a :file argument now requirements an absolute path to the file.

On another note I believe the sass-ruby library has been deprecated and upgrading to higher versions of sass-rails now depends on sassc-ruby library see: https://github.com/rails/sass-rails/releases/tag/v6.0.0. This library changes the name space of Sass to SassC and there may not be sass feature rendering parity with previous Sass version see: http://sass-compatibility.github.io/. Hence, I made a lock of the sass-rails version to ~> 5.0.8 to avoid potential breaking changes. I'd recommended doing a version bump when upgrading that just to be extra cautious.

@arlando arlando changed the title Add Rails 6 Supports Add Rails 6 Support May 15, 2020
@billhorsman
Copy link
Copy Markdown
Owner

Hey @arlando

Thanks for the contribution 👏 I like the look of this. Let me get my head round the compatability issues with Rails < 6 and give you a better response as soon as I can.

@billhorsman
Copy link
Copy Markdown
Owner

@arlando I played around with this a bit at the weekend but ran out of time. It might be that to support Rails 6 we have to drop support for some earlier versions of Rails and Ruby. It would be good to mimimise that as much as possible. Worst case, we have a new major version of this gem that only supports Rails 6 but I'd like to avoid that if possible.

@arlando
Copy link
Copy Markdown
Author

arlando commented May 18, 2020

@billhorsman I'd like to avoid that too.

Rails 6 changed the arity of the _layout method. If we follow the pattern here we should be able to maintain backwards compatibility.

In Rails 6 the sass parser has changed. sass_rails is using sassc-ruby which changed the reference of Sass to SassC. If we can dynamically detect if inline_styles_mailer should use Sass or SassC here, I think we should be able to maintain support for previous versions of Rails. But, because the sass library changed there could be regressions.

ruby wise, Rails 6 depends on versions 2.5.0 of ruby and above. I wasn't able to find any regressions between 2.2.2 (Rails 5 base line support) and 2.5.0 ruby versions. Do you think there would be some code in inline_styles_mailer that would no longer work in older or newer versions of ruby? I wasn't able to detect this.

@billhorsman
Copy link
Copy Markdown
Owner

@arlando I think your arity checks are fine, it's just the gem dependencies that worry me — as can be seen in the Travis test results.

@arlando
Copy link
Copy Markdown
Author

arlando commented May 18, 2020

@billhorsman I'm new to this 😄 so there may be a smarter way of handling this. I will look into this more soon.

@billhorsman
Copy link
Copy Markdown
Owner

@arlando I'm not new to this but that doesn't make me smarter :) The most recent test results are here.

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.

2 participants