bridgetownrb / bridgetown

A next-generation progressive site generator & fullstack framework, powered by Ruby
https://www.bridgetownrb.com
MIT License
1.13k stars 114 forks source link

Converter plugin without initializer #769

Closed akarzim closed 1 year ago

akarzim commented 1 year ago

I have recently created a converter plugin for Bridgetown without any configuration. Due to the lack of a call to Bridgetown.initializer, this log appears in development environment:

[Bridgetown]       Initializing: The `bridgetown_markdown_lazylinks' initializer could not be found

Bridgetown Version: 1.2.0

To Reproduce

  1. Start a new Bridgetown project
  2. Run bin/bridgetown apply https://github.com/akarzim/bridgetown_markdown_lazylinks
  3. Run bin/bridgetown start.

Current behavior

The bin/bridgetown start logs complain about the lack of a plugin initializer.

Expected behavior

If the plugin does not declare an initializer, Bridgetown should not try to call one.

Screenshots

❯ bundle exec bin/bridgetown start
[Bridgetown]           Starting: Bridgetown v1.2.0 (codename "Bonny Slope")
[Bridgetown]       Initializing: The `bridgetown_markdown_lazylinks' initializer could not be found
[Server] * Puma version: 5.6.5 (ruby 3.2.2-p53) ("Birdie's Version")
[Server] * PID: 6716
[Bridgetown]       Initializing: The `bridgetown_markdown_lazylinks' initializer could not be found
[Server] * Listening on http://0.0.0.0:4000
[Server] Use Ctrl-C to stop
[Frontend] yarn run esbuild-dev
[Frontend] yarn run v1.22.19
[Frontend] $ node esbuild.config.js --watch
[Frontend] esbuild: frontend bundling started...
[Bridgetown]        Environment: development
[Bridgetown]             Source: /Users/␦␦␦/␦␦␦/src
[Bridgetown]        Destination: /Users/␦␦␦/␦␦␦/output
[Bridgetown]     Custom Plugins: /Users/␦␦␦/␦␦␦/plugins
[Bridgetown]         Generating… 
[Bridgetown]    Bridgetown Feed: Generating feed for posts
[Bridgetown]         Pagination: Complete, processed 10 pagination page(s)
[Frontend] esbuild: frontend bundling complete!
[Frontend] esbuild: entrypoints processed:
[Frontend]          - index.6X27IUHB.js: 153B
[Frontend]          - index.ZVKQQ75W.css: 20.75KB
[Bridgetown]             Done! 🎉 Completed in less than 4.96 seconds.
[Bridgetown]                     
[Bridgetown]     Now serving at: http://localhost:4000
[Bridgetown]                     http://192.168.0.27:4000
[Bridgetown]                     

Computing environment (please complete the following information):

Proposals

Option 1: mandatory initializer declaration with a block

# lib/bridgetown_markdown_lazylinks.rb
# frozen_string_literal: true

require "bridgetown"
require "bridgetown_markdown_lazylinks/converter"

# @param config [Bridgetown::Configuration::ConfigurationDSL]
Bridgetown.initializer :bridgetown_markdown_lazylinks do |_|
end

Option 2: mandatory initializer declaration but make the block optional

# lib/bridgetown_markdown_lazylinks.rb
# frozen_string_literal: true

require "bridgetown"
require "bridgetown_markdown_lazylinks/converter"

# @param config [Bridgetown::Configuration::ConfigurationDSL]
Bridgetown.initializer :bridgetown_markdown_lazylinks
diff --git a/bridgetown-core/lib/bridgetown-core/configuration/configuration_dsl.rb b/bridgetown-core/lib/bridgetown-core/configuration/configuration_dsl.rb
index fcd3819..2082cda 100644
--- a/bridgetown-core/lib/bridgetown-core/configuration/configuration_dsl.rb
+++ b/bridgetown-core/lib/bridgetown-core/configuration/configuration_dsl.rb
@@ -27,7 +27,11 @@ def init(name, require_gem: true, require_initializer: true, **kwargs, &block) #
         end

         Bridgetown.logger.debug "Initializing:", name
-        @scope.initializers[name.to_sym].block.(self, **@scope.init_params[name].symbolize_keys)
+
+        if !!@scope.initializers[name.to_sym].block
+          @scope.initializers[name.to_sym].block.(self, **@scope.init_params[name].symbolize_keys)
+        end
+
         initializer.completed = true
       end

But it makes no sense to initialise a plugin without initialising anything. And this would lead to a crash when calling the bin/bridgetown plugins list command:

❯ bin/bridgetown plugins list
Registered Plugins: 7
                    bridgetown_markdown_lazylinks (Initializer)
  Exception raised: NoMethodError
undefined method `source_location' for nil:NilClass 
                 1: /Users/␦␦␦/␦␦␦/bridgetown/bridgetown-core/lib/bridgetown-core/commands/plugins.rb:43:in `block in list'
                 2: /Users/␦␦␦/␦␦␦/bridgetown/bridgetown-core/lib/bridgetown-core/commands/plugins.rb:36:in `each'
                 3: /Users/␦␦␦/␦␦␦/bridgetown/bridgetown-core/lib/bridgetown-core/commands/plugins.rb:36:in `list'
                 4: /Users/␦␦␦/.rbenv/versions/2.7.0/vendor/bundle/ruby/3.2.0/gems/thor-1.2.2/lib/thor/command.rb:27:in `run'
                 5: /Users/␦␦␦/.rbenv/versions/2.7.0/vendor/bundle/ruby/3.2.0/gems/thor-1.2.2/lib/thor/invocation.rb:127:in `invoke_command'
         Backtrace: Use the --trace option for complete information.

Option 3: optional initializer declaration

# lib/bridgetown_markdown_lazylinks.rb
# frozen_string_literal: true

require "bridgetown"
require "bridgetown_markdown_lazylinks/converter"
diff --git a/bridgetown-core/lib/bridgetown-core/configuration/configuration_dsl.rb b/bridgetown-core/lib/bridgetown-core/configuration/configuration_dsl.rb
index fcd3819..ce05a7a 100644
--- a/bridgetown-core/lib/bridgetown-core/configuration/configuration_dsl.rb
+++ b/bridgetown-core/lib/bridgetown-core/configuration/configuration_dsl.rb
@@ -14,20 +14,24 @@ def init(name, require_gem: true, require_initializer: true, **kwargs, &block) #
           name: name, require_gem: require_gem, require_initializer: require_initializer
         )

-        return unless initializer.nil? || initializer.completed == false
+        if block_given? && initializer.nil?
+          Bridgetown.logger.warn("Initializing:",
+                                 "The `#{name}' initializer could not be found")
+          return
+        end
+
+        return if initializer.nil? || !!initializer.completed

         set :init_params do
           block ? set(name, &block) : set(name, kwargs)
         end

-        if initializer.nil?
-          Bridgetown.logger.warn("Initializing:",
-                                 "The `#{name}' initializer could not be found")
-          return
+        Bridgetown.logger.debug "Initializing:", name
+
+        if !!@scope.initializers[name.to_sym].block
+          @scope.initializers[name.to_sym].block.(self, **@scope.init_params[name].symbolize_keys)
         end

-        Bridgetown.logger.debug "Initializing:", name
-        @scope.initializers[name.to_sym].block.(self, **@scope.init_params[name].symbolize_keys)
         initializer.completed = true
       end

This requires to fix bin/bridgetown plugins list command too:

diff --git a/bridgetown-core/lib/bridgetown-core/commands/plugins.rb b/bridgetown-core/lib/bridgetown-core/commands/plugins.rb
index 0e499aa..3f37878 100644
--- a/bridgetown-core/lib/bridgetown-core/commands/plugins.rb
+++ b/bridgetown-core/lib/bridgetown-core/commands/plugins.rb
@@ -39,9 +39,12 @@ def list

           if plugin.is_a?(Bridgetown::Configuration::Initializer)
             Bridgetown.logger.info("", plugin_desc)
-            Bridgetown.logger.debug(
-              "", "PATH: " + plugin.block.source_location[0]
-            )
+
+            unless plugin.block.nil?
+              Bridgetown.logger.debug(
+                "", "PATH: " + plugin.block.source_location[0]
+              )
+            end
           elsif plugin.is_a?(Bundler::StubSpecification) || plugin.is_a?(Gem::Specification)
             Bridgetown.logger.info("", "#{plugin.name} (Rubygem)")
             Bridgetown.logger.debug(

I'm not sure which option would suit you best. Let me know and I'll link a PR to this issue!

jaredcwhite commented 1 year ago

@akarzim I would prefer "Option 1: mandatory initializer declaration with a block". Just because your plugin currently doesn't require any init configuration now doesn't mean it won't in the future, and I think keeping the convention that it's mandatory is good for the ecosystem. Thanks.