Warn about deprecated element methods #649
Conversation
| def capture_stderr(&block) | ||
| captured_stream = StringIO.new | ||
| original_stream = $stderr | ||
| $stderr = captured_stream | ||
|
|
||
| block.call | ||
|
|
||
| captured_stream.string.strip | ||
| ensure | ||
| $stderr = original_stream | ||
| end |
There was a problem hiding this comment.
I added this as a module because I think it can be useful for other deprecation tests in the future.
| @@ -35,6 +35,10 @@ def register_element(method_name, tag: nil, deprecated: false) | |||
| # frozen_string_literal: true | |||
|
|
|||
| def #{method_name}(**attributes, &block) | |||
| if #{deprecated} | |||
| Kernel.warn("`#{method_name}` and `_#{method_name}` are deprecated and will be unsupported in Phlex 2.0. This HTML element is no longer recommended. Check out MDN web docs: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/#{tag}.") | |||
There was a problem hiding this comment.
-
I couldn't get called method name here (eg.
paramor_param), using__method__or__callee__returnsregister_elementhere orregister_void_elementon line 82. -
I'm not sure if it makes sense to add link to MDN docs, but it explains why the method is being deprecated and redirects the user to more detailed context.
| describe "Elements deprecation" do | ||
| deprecated_methods = %i[param] | ||
|
|
||
| deprecated_methods.each do |method_name| |
There was a problem hiding this comment.
I'm not sure if it is worth testing non deprecated elements. It would be something like:
[
Phlex::HTML::StandardElements.registered_elements,
Phlex::HTML::VoidElements.registered_elements
].each do |element_group|
element_group.each do |method_name, tag|
if deprecated_methods.include?(method_name)
# tests for method_name and _method_name
... warns about deprecation
else
# tests for method_name and _method_name
... doesn't warn about deprecation
end
end
end
|
Sorry @stephannv, I wanted to go with a simpler solution to this where the message was just passed to the I really appreciate the effort you put into this PR, but it didn’t feel right for what this is, especially as this is probably the only time we'll have to deprecate elements. |
|
No problem @joeldrapper. |
Fix first item from #632