1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Favor public_folder: true over public_*

Adding all those `public_*` methods is a bit heavy handed, we can change the API to instead  use `public_folder: true`. Change was pretty easy since it was already implemented that way.
This commit is contained in:
schneems 2016-08-21 14:08:24 -05:00
parent a22164a282
commit b47970294e
3 changed files with 47 additions and 169 deletions

View file

@ -193,13 +193,6 @@ module ActionView
}.merge!(options.symbolize_keys)) }.merge!(options.symbolize_keys))
end end
# Returns a link tag for a favicon asset in the public
# folder. This uses +favicon_link_tag+ and skips any asset
# lookups by assuming any assets are in the `public` folder.
def public_favicon_link_tag(source="favicon.ico", options={})
favicon_link_tag(source, { public_folder: true }.merge!(options))
end
# Returns an HTML image tag for the +source+. The +source+ can be a full # Returns an HTML image tag for the +source+. The +source+ can be a full
# path or a file. # path or a file.
# #
@ -244,13 +237,6 @@ module ActionView
tag("img", options) tag("img", options)
end end
# Returns an HTML image tag for the source asset in the public
# folder. This uses +image_tag+ and skips any asset
# lookups by assuming any assets are in the `public` folder.
def public_image_tag(source, options={})
image_tag(source, { public_folder: true }.merge!(options))
end
# Returns a string suitable for an HTML image tag alt attribute. # Returns a string suitable for an HTML image tag alt attribute.
# The +src+ argument is meant to be an image file path. # The +src+ argument is meant to be an image file path.
# The method removes the basename of the file path and the digest, # The method removes the basename of the file path and the digest,
@ -314,20 +300,12 @@ module ActionView
options = sources.extract_options!.symbolize_keys options = sources.extract_options!.symbolize_keys
public_poster_folder = options.delete(:public_poster_folder) public_poster_folder = options.delete(:public_poster_folder)
sources << options sources << options
multiple_sources_tag("video", sources) do |options| multiple_sources_tag_builder("video", sources) do |options|
options[:poster] = path_to_image(options[:poster], public_folder: public_poster_folder) if options[:poster] options[:poster] = path_to_image(options[:poster], public_folder: public_poster_folder) if options[:poster]
options[:width], options[:height] = extract_dimensions(options.delete(:size)) if options[:size] options[:width], options[:height] = extract_dimensions(options.delete(:size)) if options[:size]
end end
end end
# Returns an HTML video tag for the source asset in the public
# folder. This uses +video_tag+ and skips any asset
# lookups by assuming any assets are in the `public` folder.
def public_video_tag(*sources)
options = sources.extract_options!
video_tag(*sources, { public_folder: true , public_poster_folder: true }.merge!(options))
end
# Returns an HTML audio tag for the +source+. # Returns an HTML audio tag for the +source+.
# The +source+ can be full path or file that exists in # The +source+ can be full path or file that exists in
# your public audios directory. # your public audios directory.
@ -341,18 +319,11 @@ module ActionView
# audio_tag("sound.wav", "sound.mid") # audio_tag("sound.wav", "sound.mid")
# # => <audio><source src="/audios/sound.wav" /><source src="/audios/sound.mid" /></audio> # # => <audio><source src="/audios/sound.wav" /><source src="/audios/sound.mid" /></audio>
def audio_tag(*sources) def audio_tag(*sources)
multiple_sources_tag("audio", sources) multiple_sources_tag_builder("audio", sources)
end
# Returns an HTML audio tag for the source asset in the public
# folder. This uses +audio_tag+ and skips any asset
def public_audio_tag(*sources)
options = sources.extract_options!
audio_tag(*sources, { public_folder: true }.merge!(options))
end end
private private
def multiple_sources_tag(type, sources) def multiple_sources_tag_builder(type, sources)
options = sources.extract_options!.symbolize_keys options = sources.extract_options!.symbolize_keys
public_folder = options.delete(:public_folder) public_folder = options.delete(:public_folder)
sources.flatten! sources.flatten!

View file

@ -167,14 +167,6 @@ module ActionView
end end
alias_method :path_to_asset, :asset_path # aliased to avoid conflicts with an asset_path named route alias_method :path_to_asset, :asset_path # aliased to avoid conflicts with an asset_path named route
# Computes the path to an asset in the public folder.
# This uses +asset_path+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_asset_path(source, options = {})
path_to_asset(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_asset, :public_asset_path # aliased to avoid conflicts with an public_asset_path named route
# Computes the full URL to an asset in the public directory. This # Computes the full URL to an asset in the public directory. This
# will use +asset_path+ internally, so most of their behaviors # will use +asset_path+ internally, so most of their behaviors
# will be the same. If :host options is set, it overwrites global # will be the same. If :host options is set, it overwrites global
@ -190,14 +182,6 @@ module ActionView
end end
alias_method :url_to_asset, :asset_url # aliased to avoid conflicts with an asset_url named route alias_method :url_to_asset, :asset_url # aliased to avoid conflicts with an asset_url named route
# Computes the full URL to an asset in the public folder.
# This uses +asset_url+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_asset_url(source, options = {})
url_to_asset(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_asset, :public_asset_path # aliased to avoid conflicts with a public_asset_path named route
ASSET_EXTENSIONS = { ASSET_EXTENSIONS = {
javascript: ".js", javascript: ".js",
stylesheet: ".css" stylesheet: ".css"
@ -284,14 +268,6 @@ module ActionView
end end
alias_method :path_to_javascript, :javascript_path # aliased to avoid conflicts with a javascript_path named route alias_method :path_to_javascript, :javascript_path # aliased to avoid conflicts with a javascript_path named route
# Computes the path to a javascript asset in the public folder.
# This uses +javascript_path+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_javascript_path(source, options = {})
path_to_javascript(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_javascript, :public_javascript_path # aliased to avoid conflicts with a public_javascript_path named route
# Computes the full URL to a JavaScript asset in the public javascripts directory. # Computes the full URL to a JavaScript asset in the public javascripts directory.
# This will use +javascript_path+ internally, so most of their behaviors will be the same. # This will use +javascript_path+ internally, so most of their behaviors will be the same.
# Since +javascript_url+ is based on +asset_url+ method you can set :host options. If :host # Since +javascript_url+ is based on +asset_url+ method you can set :host options. If :host
@ -304,14 +280,6 @@ module ActionView
end end
alias_method :url_to_javascript, :javascript_url # aliased to avoid conflicts with a javascript_url named route alias_method :url_to_javascript, :javascript_url # aliased to avoid conflicts with a javascript_url named route
# Computes the full URL to a javascript asset in the public folder.
# This uses +javascript_url+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_javascript_url(source, options = {})
url_to_javascript(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_javascript, :public_javascript_path # aliased to avoid conflicts with a public_javascript_path named route
# Computes the path to a stylesheet asset in the public stylesheets directory. # Computes the path to a stylesheet asset in the public stylesheets directory.
# If the +source+ filename has no extension, .css will be appended (except for explicit URIs). # If the +source+ filename has no extension, .css will be appended (except for explicit URIs).
# Full paths from the document root will be passed through. # Full paths from the document root will be passed through.
@ -327,14 +295,6 @@ module ActionView
end end
alias_method :path_to_stylesheet, :stylesheet_path # aliased to avoid conflicts with a stylesheet_path named route alias_method :path_to_stylesheet, :stylesheet_path # aliased to avoid conflicts with a stylesheet_path named route
# Computes the path to a stylesheet asset in the public folder.
# This uses +stylesheet_path+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_stylesheet_path(source, options = {})
path_to_stylesheet(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_stylesheet, :public_stylesheet_path # aliased to avoid conflicts with a public_stylesheet_path named route
# Computes the full URL to a stylesheet asset in the public stylesheets directory. # Computes the full URL to a stylesheet asset in the public stylesheets directory.
# This will use +stylesheet_path+ internally, so most of their behaviors will be the same. # This will use +stylesheet_path+ internally, so most of their behaviors will be the same.
# Since +stylesheet_url+ is based on +asset_url+ method you can set :host options. If :host # Since +stylesheet_url+ is based on +asset_url+ method you can set :host options. If :host
@ -347,14 +307,6 @@ module ActionView
end end
alias_method :url_to_stylesheet, :stylesheet_url # aliased to avoid conflicts with a stylesheet_url named route alias_method :url_to_stylesheet, :stylesheet_url # aliased to avoid conflicts with a stylesheet_url named route
# Computes the full URL to a stylesheet asset in the public folder.
# This uses +stylesheet_url+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_stylesheet_url(source, options = {})
url_to_stylesheet(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_stylesheet, :public_stylesheet_path # aliased to avoid conflicts with a public_stylesheet_path named route
# Computes the path to an image asset. # Computes the path to an image asset.
# Full paths from the document root will be passed through. # Full paths from the document root will be passed through.
# Used internally by +image_tag+ to build the image path: # Used internally by +image_tag+ to build the image path:
@ -373,14 +325,6 @@ module ActionView
end end
alias_method :path_to_image, :image_path # aliased to avoid conflicts with an image_path named route alias_method :path_to_image, :image_path # aliased to avoid conflicts with an image_path named route
# Computes the path to a image asset in the public folder.
# This uses +image_path+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_image_path(source, options = {})
path_to_image(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_image, :public_image_path # aliased to avoid conflicts with a public_image_path named route
# Computes the full URL to an image asset. # Computes the full URL to an image asset.
# This will use +image_path+ internally, so most of their behaviors will be the same. # This will use +image_path+ internally, so most of their behaviors will be the same.
# Since +image_url+ is based on +asset_url+ method you can set :host options. If :host # Since +image_url+ is based on +asset_url+ method you can set :host options. If :host
@ -393,14 +337,6 @@ module ActionView
end end
alias_method :url_to_image, :image_url # aliased to avoid conflicts with an image_url named route alias_method :url_to_image, :image_url # aliased to avoid conflicts with an image_url named route
# Computes the full URL to a image asset in the public folder.
# This uses +image_url+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_image_url(source, options = {})
url_to_image(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_image, :public_image_path # aliased to avoid conflicts with a public_image_path named route
# Computes the path to a video asset in the public videos directory. # Computes the path to a video asset in the public videos directory.
# Full paths from the document root will be passed through. # Full paths from the document root will be passed through.
# Used internally by +video_tag+ to build the video path. # Used internally by +video_tag+ to build the video path.
@ -415,14 +351,6 @@ module ActionView
end end
alias_method :path_to_video, :video_path # aliased to avoid conflicts with a video_path named route alias_method :path_to_video, :video_path # aliased to avoid conflicts with a video_path named route
# Computes the path to a video asset in the public folder.
# This uses +video_path+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_video_path(source, options = {})
path_to_video(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_video, :public_video_path # aliased to avoid conflicts with a public_video_path named route
# Computes the full URL to a video asset in the public videos directory. # Computes the full URL to a video asset in the public videos directory.
# This will use +video_path+ internally, so most of their behaviors will be the same. # This will use +video_path+ internally, so most of their behaviors will be the same.
# Since +video_url+ is based on +asset_url+ method you can set :host options. If :host # Since +video_url+ is based on +asset_url+ method you can set :host options. If :host
@ -435,14 +363,6 @@ module ActionView
end end
alias_method :url_to_video, :video_url # aliased to avoid conflicts with an video_url named route alias_method :url_to_video, :video_url # aliased to avoid conflicts with an video_url named route
# Computes the full URL to a video asset in the public folder.
# This uses +video_url+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_video_url(source, options = {})
url_to_video(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_video, :public_video_path # aliased to avoid conflicts with a public_video_path named route
# Computes the path to an audio asset in the public audios directory. # Computes the path to an audio asset in the public audios directory.
# Full paths from the document root will be passed through. # Full paths from the document root will be passed through.
# Used internally by +audio_tag+ to build the audio path. # Used internally by +audio_tag+ to build the audio path.
@ -457,14 +377,6 @@ module ActionView
end end
alias_method :path_to_audio, :audio_path # aliased to avoid conflicts with an audio_path named route alias_method :path_to_audio, :audio_path # aliased to avoid conflicts with an audio_path named route
# Computes the path to a audio asset in the public folder.
# This uses +audio_path+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_audio_path(source, options = {})
path_to_audio(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_audio, :public_audio_path # aliased to avoid conflicts with a public_audio_path named route
# Computes the full URL to an audio asset in the public audios directory. # Computes the full URL to an audio asset in the public audios directory.
# This will use +audio_path+ internally, so most of their behaviors will be the same. # This will use +audio_path+ internally, so most of their behaviors will be the same.
# Since +audio_url+ is based on +asset_url+ method you can set :host options. If :host # Since +audio_url+ is based on +asset_url+ method you can set :host options. If :host
@ -477,14 +389,6 @@ module ActionView
end end
alias_method :url_to_audio, :audio_url # aliased to avoid conflicts with an audio_url named route alias_method :url_to_audio, :audio_url # aliased to avoid conflicts with an audio_url named route
# Computes the full URL to a audio asset in the public folder.
# This uses +audio_url+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_audio_url(source, options = {})
url_to_audio(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_audio, :public_audio_path # aliased to avoid conflicts with a public_audio_path named route
# Computes the path to a font asset. # Computes the path to a font asset.
# Full paths from the document root will be passed through. # Full paths from the document root will be passed through.
# #
@ -498,14 +402,6 @@ module ActionView
end end
alias_method :path_to_font, :font_path # aliased to avoid conflicts with an font_path named route alias_method :path_to_font, :font_path # aliased to avoid conflicts with an font_path named route
# Computes the path to a font asset in the public folder.
# This uses +font_path+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_font_path(source, options = {})
path_to_font(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_font, :public_font_path # aliased to avoid conflicts with a public_font_path named route
# Computes the full URL to a font asset. # Computes the full URL to a font asset.
# This will use +font_path+ internally, so most of their behaviors will be the same. # This will use +font_path+ internally, so most of their behaviors will be the same.
# Since +font_url+ is based on +asset_url+ method you can set :host options. If :host # Since +font_url+ is based on +asset_url+ method you can set :host options. If :host
@ -517,14 +413,6 @@ module ActionView
url_to_asset(source, { type: :font }.merge!(options)) url_to_asset(source, { type: :font }.merge!(options))
end end
alias_method :url_to_font, :font_url # aliased to avoid conflicts with an font_url named route alias_method :url_to_font, :font_url # aliased to avoid conflicts with an font_url named route
# Computes the full URL to a font asset in the public folder.
# This uses +font_url+ and skips any asset lookups by assuming the asset is in the
# `public` folder.
def public_font_url(source, options = {})
url_to_font(source, { public_folder: true }.merge!(options))
end
alias_method :path_to_public_font, :public_font_path # aliased to avoid conflicts with a public_font_url named route
end end
end end
end end

View file

@ -72,23 +72,23 @@ module ApplicationTests
test "public path and tag methods are not over-written by the asset pipeline" do test "public path and tag methods are not over-written by the asset pipeline" do
contents = "doesnotexist" contents = "doesnotexist"
cases = { cases = {
public_asset_path: %r{/#{ contents }}, asset_path: %r{/#{ contents }},
public_image_path: %r{/images/#{ contents }}, image_path: %r{/images/#{ contents }},
public_video_path: %r{/videos/#{ contents }}, video_path: %r{/videos/#{ contents }},
public_audio_path: %r{/audios/#{ contents }}, audio_path: %r{/audios/#{ contents }},
public_font_path: %r{/fonts/#{ contents }}, font_path: %r{/fonts/#{ contents }},
public_javascript_path: %r{/javascripts/#{ contents }}, javascript_path: %r{/javascripts/#{ contents }},
public_stylesheet_path: %r{/stylesheets/#{ contents }}, stylesheet_path: %r{/stylesheets/#{ contents }},
public_image_tag: %r{<img src="/images/#{ contents }"}, image_tag: %r{<img src="/images/#{ contents }"},
public_favicon_link_tag: %r{<link rel="shortcut icon" type="image/x-icon" href="/images/#{ contents }" />}, favicon_link_tag: %r{<link rel="shortcut icon" type="image/x-icon" href="/images/#{ contents }" />},
public_stylesheet_link_tag: %r{<link rel="stylesheet" media="screen" href="/stylesheets/#{ contents }.css" />}, stylesheet_link_tag: %r{<link rel="stylesheet" media="screen" href="/stylesheets/#{ contents }.css" />},
public_javascript_include_tag: %r{<script src="/javascripts/#{ contents }.js">}, javascript_include_tag: %r{<script src="/javascripts/#{ contents }.js">},
public_audio_tag: %r{<audio src="/audios/#{ contents }"></audio>}, audio_tag: %r{<audio src="/audios/#{ contents }"></audio>},
public_video_tag: %r{<video src="/videos/#{ contents }"></video>} video_tag: %r{<video src="/videos/#{ contents }"></video>}
} }
cases.each do |(view_method, tag_match)| cases.each do |(view_method, tag_match)|
app_file "app/views/posts/index.html.erb", "<%= #{ view_method } '#{contents}' %>" app_file "app/views/posts/index.html.erb", "<%= #{ view_method } '#{contents}', public_folder: true %>"
app "development" app "development"
@ -104,17 +104,17 @@ module ApplicationTests
test "public url methods are not over-written by the asset pipeline" do test "public url methods are not over-written by the asset pipeline" do
contents = "doesnotexist" contents = "doesnotexist"
cases = { cases = {
public_asset_url: %r{http://example.org/#{ contents }}, asset_url: %r{http://example.org/#{ contents }},
public_image_url: %r{http://example.org/images/#{ contents }}, image_url: %r{http://example.org/images/#{ contents }},
public_video_url: %r{http://example.org/videos/#{ contents }}, video_url: %r{http://example.org/videos/#{ contents }},
public_audio_url: %r{http://example.org/audios/#{ contents }}, audio_url: %r{http://example.org/audios/#{ contents }},
public_font_url: %r{http://example.org/fonts/#{ contents }}, font_url: %r{http://example.org/fonts/#{ contents }},
public_javascript_url: %r{http://example.org/javascripts/#{ contents }}, javascript_url: %r{http://example.org/javascripts/#{ contents }},
public_stylesheet_url: %r{http://example.org/stylesheets/#{ contents }}, stylesheet_url: %r{http://example.org/stylesheets/#{ contents }},
} }
cases.each do |(view_method, tag_match)| cases.each do |(view_method, tag_match)|
app_file "app/views/posts/index.html.erb", "<%= #{ view_method } '#{contents}' %>" app_file "app/views/posts/index.html.erb", "<%= #{ view_method } '#{contents}', public_folder: true %>"
app "development" app "development"
@ -127,10 +127,29 @@ module ApplicationTests
end end
end end
test "public path methods do not use the asset pipeline" do test "{ public_folder: true } does not use the asset pipeline" do
cases = { cases = {
asset_path: /\/assets\/application-.*.\.js/, /\/assets\/application-.*.\.js/ => {},
public_asset_path: /application.js/, /application.js/ => { public_folder: true},
}
cases.each do |(tag_match, options_hash)|
app_file "app/views/posts/index.html.erb", "<%= asset_path('application.js', #{ options_hash }) %>"
app "development"
class ::PostsController < ActionController::Base ; end
get "/posts?debug_assets=true"
body = last_response.body.strip
assert_match(tag_match, body, "Expected `asset_path` with `#{ options_hash}` to produce a match to #{ tag_match }, but did not: #{ body }")
end
end
test "public_compute_asset_path does not use the asset pipeline" do
cases = {
compute_asset_path: /\/assets\/application-.*.\.js/,
public_compute_asset_path: /application.js/,
} }
cases.each do |(view_method, tag_match)| cases.each do |(view_method, tag_match)|