Refactor singleton usage

[palkan]

Context

I'm working on the application consisting of multiple engines. Some of these engines provide user interface.

We wanted to make our engines as isolated as possible (though keeping in the same monorepo); hence we needed our webpack(-er) to be isolated too: separate package.json, dev server, compilation process.

It turned out that it could be done with Webpacker if we replace the global Webpacker.instance usage with the local instance–i.e. if we "fix" dependency injection.

NOTE: this PR should considered as an experimental (though the functionality has been extracted from the production app); I'd like to discuss this approach first of all.

NOTE 2: this PR doesn't introduce any backward compatibility changes and only deals with the internals.

Related tickets

#348

What's inside?

• Compiler class refactored to use @webpacker instance instead of Webpacker (
  7a0a58c)
• Added webpacker option to DevServerProxy to make it possible to use with non-default instance (
  53c8a77)
• Added explicit chdir for scripts (webpack, webpack-dev-server) to make it possible to run not only from the app root (
  e2bc236)
• Added current_webpacker to Helper to make it possible to override the default Webpacker (
  aa2aabd)
• Added env_prefix config param to dev_server section of webpacker.yml to make it possible to use different env variable for different webpackers (32fd72e)

Usage example

Suppose that we have a Rails engine MyEngine.

To add an isolated Webpacker configuration to that gem we need:

• Add config/webpacker.yml and config/webpack/*.js files
• Add bin/webpack and bin/webpack-dev-server files
• Configure engine's instance:

module MyEngine
  ROOT_PATH = Pathname.new(File.join(__dir__, ".."))

  class << self
    def webpacker
      @webpacker ||= ::Webpacker::Instance.new(
        root_path: ROOT_PATH,
        config_path: ROOT_PATH.join("config/webpacker.yml")
      )
    end
  end
end

• Configure dev server proxy

module MyEngine
  class Engine < ::Rails::Engine
    initializer "webpacker.proxy" do |app|
        insert_middleware = begin
                            MyEngine.webpacker.config.dev_server.present?
                          rescue
                            nil
                          end
        next unless insert_middleware

        app.middleware.insert_before(
          0, "Webpacker::DevServerProxy",
          ssl_verify_none: true,
          webpacker: MyEngine.webpacker
        )
      end
  end
end

• (optional) If you have multiple webpacker, you would probably want to run multiple dev servers at a time, and hence be able to configure their setting though env vars (we do use env vars within a docker-compose dev env):

# webpacker.yml
# ...
development:
  # ...
  dev_server:
    env_prefix: "MY_ENGINE_WEBPACKER_DEV_SERVER"
    # ...

• Configure helper:

require "webpacker/helper"

module MyEngine
  module ApplicationHelper
    include ::Webpacker::Helper

    def current_webpacker
      MyEngine.webpacker
    end
  end
end

Now you can use stylesheet_pack_tag and javascript_pack_tag from within your engine.

• Add Rake task to compile assets in production (rake my_engine:webpacker:compile)

namespace :my_engine do
  namespace :webpacker do
    desc "Install deps with yarn"
    task :yarn_install do
      Dir.chdir(File.join(__dir__, "../..")) do
        system "yarn install --no-progress --production"
      end
    end

    desc "Compile JavaScript packs using webpack for production with digests"
    task compile: [:yarn_install", :environment] do
      Webpacker.with_node_env("production") do
          if MyEngine.webpacker.commands.compile
            # Successful compilation!
          else
            # Failed compilation
            exit!
          end
      end
    end
  end
end

I think, most of this work could be automated via generators in the future, but that's not the scope of this PR: here I want to slightly change the Webpacker design to make it more flexible.

[gauravtiwari]

Thanks, @palkan Is this still a WIP?

I would very much like the engines support to be in.

[palkan]

Hi @gauravtiwari,

It's not WIP from the functionality point of view (we're already using this brunch). Btw, added one more commit to make env prefix for dev server configurable (
32fd72e).

What's left then?

I think, we need a way to somehow automate the process of configuring Webpacker for an engine (all the steps I described above). Something like rails g webpacker:install but for the engine (rails g webpacker:engine:install?).

Or maybe it's better to add Webpacker support to rails plugin new?

Another thing not covered here: how to deploy the application?

We use the following trick:

# main app Rakefile
Rake::Task["assets:precompile"].enhance do
  Rake::Task["my_engine:webpacker:compile"].invoke
end

So, we don't need separate tasks to be invoked during the deployment (works perfectly on Heroku).

To sum up, from the code perspective this PR looks complete to me (that means that we're happily use this branch without additional monkey-patches).

[gauravtiwari]

I would just call Webpacker.instance directly instead of putting into a method. What do you think?

[palkan]

The goal is to make it possible to override the Webpacker instance, so in the engine's helper we do:

require "webpacker/helper"

module MyEngine
  module ApplicationHelper
    include ::Webpacker::Helper
    
    def current_webpacker
       MyEngine.webpacker # or whatever
    end
  end
end

Webpacker.instance would point to global Webpacker (i.e. ::Webpacker.instance), so that won't work

[gauravtiwari]

Thanks, @palkan This is great 👍

Perhaps, make a changelog entry with experimental note and link to this PR. We can work on another PR to add installation support either via plugin or rake task. Add relevant documentation.

[gauravtiwari]

Could you please rebase with the master branch?

[palkan]

Could you please rebase with the master branch?

This branch is based on 3-x-stable. Would you like me to add a new PR for v4?

[mathewbaltes]

Probably would be best to have this for v4.

[palkan]

Probably would be best to have this for v4.

Absolutely.

Right now I'm migrating our app to v4) I'll open a new PR as soon as I test this feature for v4 in prod/dev.

[palkan]

Closed in favour of #1836