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

[LazyBlock] Allows using complex expressions with configuration #1931

Merged
merged 6 commits into from
Nov 14, 2019

Conversation

myxoh
Copy link
Member

@myxoh myxoh commented Nov 14, 2019

This PR is motivated by a bug I've introduced on our system.
The problem, when you are trying to waterfall a configuration, is that LazyValue doesn't play nicely inside an expression so this code

value = configuration[:attribute] || 'default_value' 

Is evaluated when the class is load as: configuration[:attribute] => LazyValue || 'default_value' => LazyValue
Which means that even when you mount the API with attribute: nil the value will equal nil rather than 'default_value' as a developer might expect.

To mitigate this, I've introduced a lazy block, so you are able to put that expression within a mounted block which will only be evaluated when the API is mounted, and furthermore, will prevent methods using the response of the expression to running before the API is mounted

@dblock
Copy link
Member

dblock commented Nov 14, 2019

👍

@dblock dblock merged commit c3de184 into ruby-grape:master Nov 14, 2019
@dblock
Copy link
Member

dblock commented Nov 14, 2019

Merged

basjanssen pushed a commit to basjanssen/grape that referenced this pull request Feb 28, 2020
…-grape#1931)

* Introduces the concept of a LazyBlock which gets excecuted on mount

* Fixes LazyBlock within a namespace

* Adds more specs so that given can also be used for conditional lazy blocks

* Documents expressions

* Adds a changelog

* Update README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants