Require runtime_dependencies of a Gem-based theme from its .gemspec file
#5914
Conversation
runtime_dependencies of a Gem-based theme from its .gemspec file
|
Requesting a restart of the Travis build for this PR.. |
|
I couldn't load the page to trigger a restart... Try pushing another commit? |
okay.. |
|
label: |
lib/jekyll/plugin_manager.rb
Outdated
| end | ||
| dependencies.each do |dep| | ||
| External.require_with_graceful_fail(dep.name) unless dep.name == "jekyll" | ||
| end |
There was a problem hiding this comment.
You could simplify this to
site.theme.runtime_dependencies.each do |dep|
next if dep.name == "jekyll"
External.require_with_graceful_fail(dep.name) if plugin_allowed?(dep.name)
end
lib/jekyll/plugin_manager.rb
Outdated
| @@ -29,6 +30,15 @@ def require_gems | |||
| ) | |||
| end | |||
|
|
|||
| def require_theme_deps | |||
| if site.theme.runtime_dependencies | |||
| site.theme.runtime_dependencies.each do |dep| | |||
There was a problem hiding this comment.
We tend to prefer guard block style, which is:
def require_theme_deps
return false unless site.theme && site.theme.runtime_dependencies
# ... do the actual work, expecting the above variables to exist
endYou could alternatively use #Array(), which will create an empty array if the value is nil:
def require_theme_deps
Array(site.theme.runtime_dependencies).each do |dep|
# ...
end
endThere was a problem hiding this comment.
I gez I'll go with the explicit return style instead of the alternative wherein a block is called even on an empty array.
|
@pathawks Any objections to the idea here? |
docs/_docs/themes.md
Outdated
| From `v3.5`, Jekyll will automatically require all `whitelist`-ed `runtime_dependencies` of your theme-gem even if they're not explicitly included under the `gems` array in the site's config file. (NOTE: `whitelist`-ing is only required when `build`-ing or `serve`-ing with the `--safe` option.) | ||
|
|
||
| With this, the end-user need not keep track of the plugins required to be included in their config file for their theme-gem to work as intended. | ||
|
|
There was a problem hiding this comment.
Added a piece of documentation for this particular feature.
/cc @DirtyF
There was a problem hiding this comment.
@ashmaroli Could we please stick to plain english instead of mixing commands and dashes to ease readability?
There was a problem hiding this comment.
Sure.
Something like this: ?
From `v3.5`, Jekyll will automatically require all whitelisted `runtime_dependencies` of
your theme-gem even if they're not explicitly included under the `gems` array in the
site's config file. (NOTE: whitelisting is only required when building or serving with the
`--safe` option.)
With this, the end-user need not keep track of the plugins required to be included in their
config file for their theme-gem to work as intended.
|
I just realized that this may have an unintended side-effect: Scenario: UPDATE: Two possible solutions:
|
|
Themes should only require what they require. If a user wants something lighter weight, they can either use a lighter theme, or fork a theme and remove the cruft. |
|
@jekyllbot: merge +minor |
|
I agree with @ashmaroli here.
Here's my use case. I developed a theme for jekyll (https://github.com/tlnagy/jekyll-lab-notebook) that is more of a theme+engine. It's an electronic lab notebook engine (for biological sciences, mainly) that is a nice theme + a bunch of plugins (https://github.com/tlnagy/jekyll-lab-notebook-plugins) to make everything work well. I wanted it to be easy to set up so the theme requires the plugins gem, but now there is no way to disable the plugins. The |
|
Hi @tlnagy the consensus is that a theme should henceforth list only the very essential plugins (i.e. those without which, your theme would simply fail) as |
Have Jekyll require the
runtime_dependenciesof a theme-gem from its.gemspecfile so that the theme's consumers need not have to edit theirgemsarray to include plugins required by their theme.--
/cc @jekyll/ecosystem