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

Exposing params via needs is not congruent #853

Closed
mwlang opened this issue Jul 16, 2019 · 1 comment · Fixed by #1034
Closed

Exposing params via needs is not congruent #853

mwlang opened this issue Jul 16, 2019 · 1 comment · Fixed by #1034
Labels
clarify api Rename/remove/add something to make the API easier to understand

Comments

@mwlang
Copy link
Contributor

mwlang commented Jul 16, 2019

In the Pages, we access params with "@" whereas in the Action, we access params without the leading "@". This incongruence leads to extra effort on developer to remember when "@" is required and when not.

For example:

In the Action:

class Charts::Folder < BrowserAction
  include Auth::AllowGuests

  get "/charts/:folder" do
    render FolderPage, folder: folder
  end
end

In the Page:

class Charts::FolderPage < PublicLayout
  needs folder : String

  def content
    h1 "Folder #{@folder}"
    # ...
  end
end

The needs directive should expose params so as to not need the leading "@" and the Page definition would become:

class Charts::FolderPage < PublicLayout
  needs folder : String

  def content
    h1 "Folder #{folder}"
    # ...
  end
end
@jwoertink jwoertink added the clarify api Rename/remove/add something to make the API easier to understand label Oct 24, 2019
@paulcsmith
Copy link
Member

This is also confusing because in Operations we generate a getter. We should definitely create a getter for needs in operations as well

https://gitter.im/luckyframework/Lobby?at=5db8858e2f8a034357fc578f

@paulcsmith paulcsmith changed the title exposing params via needs is not congruent Exposing params via needs is not congruent Mar 7, 2020
paulcsmith added a commit that referenced this issue Mar 7, 2020
Closes #853

The breaking change is that you can't use ? in 'needs' anymore. Also we
may overwrite a method by generating a getter but I think there is very
little change since most methods would be defined *after* needs
paulcsmith added a commit that referenced this issue Mar 7, 2020
Closes #853

The breaking change is that you can't use ? in 'needs' anymore. Instead
we generate a 'getter?' if the type is a Bool. Also we may overwrite a
method by generating a getter but I think there is very little change
since most methods would be defined *after* needs
paulcsmith added a commit that referenced this issue Mar 7, 2020
Closes #853

The breaking change is that you can't use ? in 'needs' anymore. Instead
we generate a 'getter?' if the type is a Bool. Also we may overwrite a
method by generating a getter but I think there is very little change
since most methods would be defined *after* needs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarify api Rename/remove/add something to make the API easier to understand
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants