Mange / roadie-rails

Making HTML emails comfortable for the Rails rockstars
MIT License
367 stars 66 forks source link

Adds support for propshaft (#2) #114

Closed jvillarejo closed 6 months ago

jvillarejo commented 6 months ago

I am migrating my webapp to use propshaft and dartsass and removing sprockets as the asset pipeline.

This PR introduce a new AssetPropshaftProvider to resolve the assets instead of the AssetPipelineProvider.

Researching propshaft more I ended replicating a similar idea implemented in the Propshaft::Server class.

We need to provide roadie with the compiled stylesheet, so it can:

Now the integration_spec works without requiring the assets to be compiled. It serves the compiled assets when running in development environment (which the dummy apps run)

I also added expectations for the body `background-color: green inlining.

It fixes https://github.com/Mange/roadie-rails/issues/112

PikachuEXE commented 6 months ago

CI failed https://github.com/Mange/roadie-rails/actions/runs/9022266978/job/24797354807?pr=114

Please fix when you can~

jvillarejo commented 6 months ago

There are some files that appear on the linter when running bundle exec standardrb that I didn't change This ones:

  lib/roadie/rails/mailer.rb:11:9: Style/ArgumentsForwarding: Use anonymous block arguments forwarding (`&`).
  lib/roadie/rails/mailer.rb:13:31: Style/ArgumentsForwarding: Use anonymous block arguments forwarding (`&`).
  lib/roadie/rails/options.rb:109:23: Performance/StringIdentifierArgument: Use `:"#{option}="` instead of `"#{option}="`.

¿Should I fix them too on this PR ?

PikachuEXE commented 6 months ago

Yes please

jvillarejo commented 6 months ago

There is a problem here. standardrb mark as linter errors this ones:

lib/roadie/rails/mailer.rb:11:9: Style/ArgumentsForwarding: Use anonymous block arguments forwarding (`&`).
lib/roadie/rails/mailer.rb:13:31: Style/ArgumentsForwarding: Use anonymous block arguments forwarding (`&`).
lib/roadie/rails/options.rb:109:23: Performance/StringIdentifierArgument: Use `:"#{option}="` instead of `"#{option}="`.

Fixing the one in lib/roadie/rails/options.rb shouldn't present any problem.

But the anonymous block arguments forwarding is a feature added in ruby 3.1 If I change that, the library won't be compatible with previous ruby versions.

I don't think that's part of the PR. but I don't know why the linter is now failing.

What should we do? I can ignore the errors with a comment.

PikachuEXE commented 6 months ago

Coz the lint job is run on ruby 3.2 Please ignore with config file https://github.com/standardrb/standard#ignoring-specific-rules-in-files-and-globs Better have one place to track all ignore rules (will be removed when support for earlier ruby versions dropped)

jvillarejo commented 6 months ago

Done @PikachuEXE

I added .standard.yml file to ignore those errors: https://github.com/Mange/roadie-rails/pull/114/commits/b6d8de1930dd439b33411fd7ef5b3009a6996c0b

PikachuEXE commented 5 months ago

CI failing now due to actual path shorten I have no idea if it's the default config changed or the setup in this project's testing causing it... https://github.com/Mange/roadie-rails/actions/runs/9326040234

jvillarejo commented 5 months ago

I will check it out.

PikachuEXE commented 4 months ago

@jvillarejo Any progress?

Edit 1: https://github.com/rails/propshaft/pull/173 ?

jvillarejo commented 4 months ago

Sorry @PikachuEXE I been having some rough weeks. I had this on my TODO list.
I will try to do fix it this week. Thanks for the link !

jvillarejo commented 4 months ago

Hey @PikachuEXE I made the PR that fixes the tests: https://github.com/Mange/roadie-rails/pull/115

Again, sorry for the delay. I had some very busy weeks.

PikachuEXE commented 4 months ago

It's simply test case no urgency at all! :)