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

Declaring "needs" for a component with a Proc fails to compile #1269

Open
stephendolan opened this issue Sep 23, 2020 · 6 comments
Open

Declaring "needs" for a component with a Proc fails to compile #1269

stephendolan opened this issue Sep 23, 2020 · 6 comments
Labels
usability Features that improve framework usability

Comments

@stephendolan
Copy link
Member

Passing procs as parameters can be really helpful for named slots in Lucky components, as shown here:
https://www.youtube.com/watch?v=KoiKOD63tRQ

This episode originated from a snippet provided by @paulcsmith here:
https://gist.github.com/paulcsmith/01c1ced3e681de99d0ec343fdf57e2b7#gistcomment-3419386

Specifically, he wanted to be able to write something like:

class MyCustomComponent < BaseComponent
  needs user : User
  needs some_content : ->
  needs some_user_content : User->
  
  def render
    div do
      some_content.call
      some_user_content.call(user)
    end
  end
end

However, when using any more than one needs when any of them was a Proc, I kept getting a compile-time error of Error: undefined macro method 'ProcNotation#types. Switching to using Proc for the type worked, but Paul requested that an issue be opened to look into supporting the -> syntax.

class MyCustomComponent < BaseComponent
  needs user : User
  needs some_content : Proc(Void)
  needs some_user_content : Proc(User)
  
  def render
    div do
      some_content.call
      some_user_content.call(user)
    end
  end
end
@jwoertink
Copy link
Member

I'm not sure if this would be allowed. needs takes a type declaration, so the thing on the right of : would have to be a Type, and -> is a literal. Like doing needs password : "". But if needs content : Proc(Whatever) doesn't work, then that is definitely an issue.

@stephendolan
Copy link
Member Author

The Proc() type definitely works and compiles fin! I just had this in my backlog to create at Paul's request in that Gist:

Can you open an issue about using it with the -> syntax? I like that a bit better so might be cool to add!

@jwoertink jwoertink added the needs investigation A possible bug / better docs needed. Investigate further label Sep 24, 2020
@matthewmcgarvey
Copy link
Member

by chance, did he mean needs some_content = ->? It makes more sense to me that he was referring to a default argument that does nothing

@paulcsmith
Copy link
Member

This is what I meant and I think it should work. The -> is weird in that Crystal seems to get a type restriction from it too. For example https://play.crystal-lang.org/#/r/9r36/edit

I think this is one of the only things in Crystal where the literal works as a type restiction. It's kinda weird. Definitely not required to support it, but it would be nice IMO

@jwoertink
Copy link
Member

oh wow. I had no idea. That's a strange one for sure, but yeah, if getter supports it, I'm sure we can too!

@jwoertink jwoertink added usability Features that improve framework usability and removed needs investigation A possible bug / better docs needed. Investigate further labels May 30, 2021
@jwoertink
Copy link
Member

Looking in to this, it seems that at the macro level, -> is transformed in to (-> ) which is an instance of ProcNotation. To get the input types, you call inputs, and the return type is output.

Where this gets tricky is that -> returns an empty array for inputs. Then you also have _->_ which is valid, but the _ returns this Underscore object which I had trouble finding more info on.

So just for some pseudo code here, it seems like we'd have to do something like this:

if type_dec.type.is_a?(ProcNotation)
  if type_dec.type.inputs.empty?
  else
    type_dec.type.inputs.each do |klass|
       if klass.stringify == "_"
       else
       end
    end
  if type_dec.type.output.nil?
  elsif type_dec.type.output.stringify == "_"
  else
  end
else

end

Then basically we have to manually build out a Proc(Whatever, Whatever) and throw that inside of our needs... Then we have to consider that needs is actually defined separately in Avram than in Lucky

This is something that can probably be done post 1.0. If anyone happens to land here and really hoping for this to work, let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
usability Features that improve framework usability
Projects
None yet
Development

No branches or pull requests

4 participants