usabilityhub / rails-erb-loader

Embedded Ruby (.erb) webpack loader for Ruby projects.
MIT License
103 stars 28 forks source link

Allow changing of executable #16

Closed rhys-vdw closed 7 years ago

rhys-vdw commented 7 years ago

Executable command should be able to be changed from ./bin/rails runner <args>.

One might need to use a different relative path, such as the creator of wow-erb-loader did. wow-erb-loader is a modified copy+paste of an older version of this project.

Actually it should be fine to use ruby <args>. This would make rails-erb-loader configurable to behave like r-erb-loader. Only potential problem is that ruby doesn't come with Erubis by default.

guilleiguaran commented 7 years ago

We can support both ERB and Erubis with something like this:

begin
  require 'erubis'
  handler = Erubis::Eruby
rescue LoadError
  require 'erb'
  handler = ERB
end

I'm happy to apply the patch if it seems fine for you

rhys-vdw commented 7 years ago

Hm. I'm not sure what the best solution is here. I'd probably prefer something like:

engine = ARGV[1]
handler = case engine
  when 'erubis'
    require 'erubis'
    Erubis::Eruby
  when 'erb'
    require 'erb'
    ERB
  else raise "Unknown templating engine `#{engine}`"
end

That way the client needs to explicitly choose their preferred engine.

I'm happy to apply the patch if it seems fine for you

I'm not really sure if this is necessary yet. No harm in adding it in I guess.

rhys-vdw commented 7 years ago

This was partially address by #20, which added the rails option, allowing a different path to the rails executable.

However, running ruby directly (and therefore not requiring Rails) is not yet supported, because we always append " runner " to the command.

I think we could do something like this:

  railsErbLoader: {
    runner: 'ruby',
    engine: 'erb'
  }

Which would override defaults of:

  railsErbLoader: {
    runner: './bin/rails runner',
    engine: 'erubis'
  }

There's no real rush to get this done because nobody has asked for it explicitly, but if you're still keen @guilleiguaran then go for it.

I don't mind if we use your begin/rescue approach or my engine config suggestion. Either seems fine.

rhys-vdw commented 7 years ago

Fixed by #20, #21