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

Don't eagerly evaluate proc defaults #271

Merged
merged 3 commits into from Apr 7, 2015
Merged

Don't eagerly evaluate proc defaults #271

merged 3 commits into from Apr 7, 2015

Conversation

tfausak
Copy link
Collaborator

@tfausak tfausak commented Mar 31, 2015

Fixes #269.

This pull request makes it so that defaults don't get eagerly evaluated if they're procs. Take this interaction for example:

class Example < ActiveInteraction::Base
  object :object,
    default: -> { puts 'default'; Object.new }
end

Simply by defining this class, "default" will be output the standard out. That's a little surprising.

We eagerly evaluate defaults to make sure you didn't give us something that would never work. Since procs can change every time they are called, it doesn't make sense to eagerly evaluate them for this purpose.

@tfausak tfausak self-assigned this Mar 31, 2015
@tfausak tfausak added this to the v2.0.0 milestone Mar 31, 2015
@tfausak
Copy link
Collaborator Author

tfausak commented Apr 1, 2015

@AaronLasseigne can you review this?

@AaronLasseigne
Copy link
Owner

Will this still throw an error when it's lazily evaluated? Also, it needs a changelog entry.

@tfausak
Copy link
Collaborator Author

tfausak commented Apr 1, 2015

Yes, it will still throw an error.

class Example < ActiveInteraction::Base
  boolean :x, default: -> { 'not a boolean' }
end
Example.run
# ActiveInteraction::InvalidDefaultError: x: "not a boolean"

tfausak added a commit that referenced this pull request Apr 7, 2015
Don't eagerly evaluate proc defaults
@tfausak tfausak merged commit fa10b0d into v2.0.0 Apr 7, 2015
@tfausak tfausak deleted the gh-269 branch April 7, 2015 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants