Loading order is causing issues

[sobrinho]

This works:

gem "actionview", "6.0.3.2"
gem "rails-html-sanitizer", "1.4.3"

require "action_view"
require "rails-html-sanitizer"

include ActionView::Helpers::SanitizeHelper

puts sanitize "hello"

This doesn't:

gem "actionview", "6.0.3.2"
gem "rails-html-sanitizer", "1.4.3"

require "rails-html-sanitizer"
require "action_view"

include ActionView::Helpers::SanitizeHelper

puts sanitize "hello"

In a real world app, sidekiq loads the files in the correct order but when using sidekiqswarm it doesn't.

Although, I don't see anything special with sidekiqswarm other than its forking machinery there.

Is this something we can fix here?

The current workaround is to require the action view in a initializer to make sure it loaded:

$ cat config/initializers/00_action_view.rb
require 'action_view/helpers/sanitize_helper'

The problem is that this gem is defining the module here: https://github.com/rails/rails-html-sanitizer/blob/master/lib/rails-html-sanitizer.rb#L30-L32

And possible we are using autoloading inside rails which doesn't require the file.

[flavorjones]

Thanks for reporting this, and sorry you're having problems. I'll take a look shortly and try to reproduce what you're seeing.

[flavorjones]

@sobrinho I'd like to make sure that I'm seeing the same thing you're seeing. This is my repro script:

#! /usr/bin/env ruby

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"
  gem "actionview"
end

require "rails-html-sanitizer"
require "action_view"

class Foo
  include ActionView::Helpers::SanitizeHelper
end

puts Foo.new.sanitize("hello")

which results in:

Fetching gem metadata from https://rubygems.org/.........
Resolving dependencies...
Using concurrent-ruby 1.1.10
Using builder 3.2.4
Using minitest 5.16.3
Using erubi 1.11.0
Using racc 1.6.0
Using crass 1.0.6
Using bundler 2.3.19
Using nokogiri 1.13.9 (x86_64-linux)
Using loofah 2.19.0
Using i18n 1.12.0
Using tzinfo 2.0.5
Using rails-html-sanitizer 1.4.3
Using activesupport 7.0.4
Using rails-dom-testing 2.0.3
Using actionview 7.0.4
./144-require-order.rb:17:in `<main>': undefined method `sanitize' for #<Foo:0x00007f9d339076a8> (NoMethodError)

puts Foo.new.sanitize("hello")
            ^^^^^^^^^

but if I comment out require "rails-html-sanitizer" then I see the sanitized string "hello".

Have I reproduced what you're seeing?

[flavorjones]

I think your analysis is correct: when rails-html-sanitizer is loaded first, it defines the module which means the actual helper does not get autoloaded.

You shouldn't need to explicitly require "rails-html-sanitizer", but I'm not entirely sure of the context in which you're operating. As a workaround, can you try removing that explicit require?

[imahungrypanda]

@flavorjones That is the error we are facing too. Our app is never explicitly requiring rails-html-sanitizer.

Our current work around is that we created an initializer for action view with require "action_view/helpers/sanitize_helper"

[flavorjones]

OK. Regardless, this method of monkeypatching core classes is problematic. I'll reach out to Rails core and ask them if there is a "right" way to do this.

It might be interesting to investigate why rails-html-sanitizer is being loaded before the SanitizeHelper, if you get a few minutes to puts some backtrace it might be illuminating, because clearly something else is going wrong besides the blind monkeypatch.

[flavorjones]

After a brief chat, it seems like the right decision is to move these methods upstream into Actionview. I'll take care of that.

[flavorjones]

@rafaelfranca It looks like commit 2dbd320 (and https://github.com/rails/rails/commit/67b42cb) moved these methods out of ActionView and into this gem. Can you help me understand why they were originally moved here?

[rafaelfranca]

hmmm, let me try to remember. I think I had the impression that we might want to change implementation later on and not have specific behavior of this gem in Action View. I think we can move it back.

[flavorjones]

@sharvy expressed an interest in working on cleaning this up. Still interested?