-
Notifications
You must be signed in to change notification settings - Fork 34
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
Bug: instance variable leaks from child to parent scopes #37
Comments
This case is very interesting, as it raises a design question for Docile. BackgroundThe core design goal of Docile is to make variables, instance variables, and methods that are visible in the context in which the DSL block is written, available for usage within the DSL block. That is, for example: # context of the block, outside the DSL object
@temperature = 99
# treat an instance of Array as a DSL
dsl = []
Docile.dsl_eval_with_block_return(dsl) do
append @temperature if @temperature > 100
end
#=> [] In order to make this work for instance variables like Design problemThe above only describes the use case of reading outside instance variables inside the DSL block. Docile also supports writing instance variables inside the DSL block, and having them propagate to the outside context. That is, in order for # before
@an_instance_var
#=> nil
Docile.dsl_eval([]) do
@an_instance_var = 42
end
# after
@an_instance_var
#=> 42 The problem at hand is that, in a multi-level DSL such as found here in Potential solutionsBrainstorming (please add more)
Additional ideas are very welcome! |
Hi @ms-ati , o = Docile.dsl_eval(Object.new) { @foo = 1 }
o.instance_variables # => [@:foo] |
Thank you as always @taichi-ishitani! You voted for instance variables written in the block being written to the DSL object. Isn’t this going to remain true no matter what we do? My understanding is that the question is whether to continue copying the instance variables “out” to the block’s lexical context. Perhaps I have misunderstood, if so can you please help me to understand? |
Hi @ms-ati , Sorry for delay because I'm on year end/new year holidays.
Your understanding is correct. I think this issue is to determine basis of Docile and is very important. Therefore, I added a new behavior. I think what users want Docile to behave is depending on their contexts and it is better that Docile offer the option to control where instance variables will be written back. This thought is close to How about implementing this option as a new argument like below? Docile.dsl_eval([], readonly_ivars: true) { @foo = 1 }
Docile.dsl_eval([], writeback_ivars_to: :block_context) { @foo = 1 }
Docile.dsl_eval([], writeback_ivars_to: dsl_object) { @foo = 1 }
|
@taichi-ishitani I'd like to come to a conclusion for how to support. the goals of this ticket in a future Docile 2.0. If you'd like to write up a more complete design document explaining this as a feature request, it could be very helpful? |
See modern-project/modern-ruby@e1eee26 where Docile was replaced with
instance_eval
to avoid the leakage of an instance variable:https://github.com/modern-project/modern-ruby/blob/e1eee26187e5281f711ac3d52aaf925cf1364b31/lib/modern/dsl.rb#L10-L13
The text was updated successfully, but these errors were encountered: