Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add nicer compiler errors when assigning a parameter to type LuckyAction.class #1243

Open
stephendolan opened this issue Aug 24, 2020 · 5 comments
Labels
improve error experience Make errors nicer to deal with

Comments

@stephendolan
Copy link
Member

It seems like folks often want to write wrappers that can handle links, and will leverage the link_helper.cr definitions to figure out what to ask for in a parameter.

Something like this:

class UI::Button < BaseComponent
  needs title : String
  needs to : Lucky::Action.class
  def render
    link title, class: "btn", to: to
  end
end

Called like this:

m UI::Button, title: "Button Text", to: App::Show

Generates this cryptic compiler error:

image

Here's why, as noted in our Gitter:

https://github.com/luckyframework/lucky/blob/master/spec/lucky/with_defaults_spec.cr
The error is because it is saying that a Lucky::Action could be any number of Lucky::Actions which have various different arguments. So your component would need to handle all the different kind of actions for it to work correctly. What you could also try to do is make it generic by making the needs route : T and that may work
But I think that with_defaults is nice because you can use any tag method with it like button, submit etc.

Alllllll that context provided, I think it'd be really great if Lucky could step in and help out here when it seems that someone is trying to type-check a parameter with a Lucky::Action.class. Something like:

If you're trying to validate that a Lucky `link_helper` is provided, you should use a generic `needs param : T` or the built-in `with_defaults` helper.

The message needs some work, but the gist is that Lucky could point users towards what they want based on the use cases for checking a LuckyAction.class.

@paulcsmith
Copy link
Member

paulcsmith commented Aug 24, 2020

An example of doing this with a generic:

class UI::Button(T) < BaseComponent
  needs title : String
  needs action : T

  def render
    link title, to: action, class: "btn"
  end
end

And with_defaults (the most flexible and preferred solution)

class UI::Button < BaseComponent
  def render
    with_defaults class: "btn" do |button_builder|
      yield button_builder
    end
  end
end

# Call *any* tag method. `link`, `button`, etc.
m UI::Button, &.link("My link, to: App::Show)

@paulcsmith paulcsmith added the improve error experience Make errors nicer to deal with label Aug 24, 2020
@da1nerd
Copy link

da1nerd commented Aug 24, 2020

Although not preferred, the usage of the generic version does look a bit nicer if you aren't used to blocks. I guess it would just depend how flexible/reusable you want your component to be. To complete the example above here's the usage of the generic.

m UI::Button, title: "My Link", to: App::Show

and your with_defaults is missing a quote.

m UI::Button, &.link "My link", to: App::Show

@paulcsmith
Copy link
Member

paulcsmith commented Aug 31, 2020

@neutrinog I agree it looks nicer especially if you are new to blocks. The downside is that you can't do something like this with the generic version since it uses a different tag than an a :(

m UI::Button, &.submit("Save my form")

I think adding parens around the block helps a bit so I'm going to start doing that form now on to hopefully clarify things a bit. We'll see :

@neurodynamic
Copy link
Contributor

@paulcsmith Wrong tag I think 😛

@paulcsmith
Copy link
Member

Oops! Sorry @neurodynamic!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improve error experience Make errors nicer to deal with
Projects
None yet
Development

No branches or pull requests

4 participants