Asset helpers should not call Webpacker.dev_server.running? in production

[javan]

Webpacker.dev_server.running? opens a socket connection:

webpacker/lib/webpacker/dev_server.rb

Lines 12 to 17 in c46d751

	def running?
	Socket.tcp(host, port, connect_timeout: connect_timeout).close
	true
	rescue
	false
	end

That's fine for development mode, but no good for production.

webpacker/lib/webpacker/helper.rb

Lines 13 to 14 in c46d751

	def asset_pack_path(name, **options)
	unless Webpacker.dev_server.running? && Webpacker.dev_server.hot_module_replacing?

webpacker/lib/webpacker/helper.rb

Lines 30 to 31 in c46d751

	def asset_pack_url(name, **options)
	unless Webpacker.dev_server.running? && Webpacker.dev_server.hot_module_replacing?

webpacker/lib/webpacker/helper.rb

Lines 64 to 65 in c46d751

	def stylesheet_pack_tag(*names, **options)
	unless Webpacker.dev_server.running? && Webpacker.dev_server.hot_module_replacing?

Introduced in #641 and #1172.

/cc @dhh @gauravtiwari

[gauravtiwari]

Good spot 👍 think we need to bypass this check altogether in production or make this block development specific only?

unless Webpacker.env.development? &&  Webpacker.dev_server.running? && Webpacker.dev_server.hot_module_replacing?

[javan]

That would work, I suppose. Probably time for a new helper to encapsulate that condition. Another option is to have Webpacker::DevServer#running? return false unless env.development?.

[dhh]

Agree we shouldn’t keep piling on that expression. I vaguely remember something about the dev server being used in test env though? But having this short circuit in #running? seems right.

…

On Jan 12, 2018, at 21:25, Javan Makhmali ***@***.***> wrote: That would work, I suppose. Probably time for a new helper to encapsulate that condition. Another option is to have Webpacker::DevServer#running? return false unless env.development?. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.