Nested Conditional Context Shorthand Statements Cause Unexpected Errors

[teliosdev]

What a mouthful. This is what it boils down to:

{{# user:admin? }}
  The user {{ user:name }} is an admin!
{{/ user:admin? }}

Is an error. Instead, curly expects this:

{{# user:admin? }}
  The user {{ name }} is an admin!
{{/ user:admin? }}

That is, while inside of the conditional block, the contextual block was entered - meaning all components are resolved against the user contextual block, which is not the desired behavior.

[dasch]

Yeah – I can see why that would be counter-intuitive. The reason is that the above code is equivalent to

{{@user}}
  {{#admin?}}
    The user {{name}} is an admin!
  {{/admin?}}
{{/user}}

That is, the : syntax is really a macro. I'm not sure if it makes sense to special case the conditional blocks here – you'd expect the context to change when doing e.g. {{@user:avatar}} {{url}} {{/user:avatar}}...

[dasch]

Actually, looking at https://github.com/zendesk/curly/blob/master/lib/curly/compiler.rb#L119-L135 I'm surprised to see this behavior, so I guess it's a block. Can you write a failing test for this?

[teliosdev]

You would expect the context to change in that example because it's a "context tag" - it's entire behavior is to change the context you're currently running. However, a "conditional tag"'s behavior is to output based on a condition - not to change the context. It makes no sense for a conditional tag to have a side effect of changing the context.

This branch contains a failing test for it.

[teliosdev]

As a note, when {{#item:hello?}}{{value}}{{/item:hello?}} is encountered, it is my understanding that curly emits something like this: (context item (conditional hello? (component value))), meaning that (component value) is resolved against the item context - since, on lines 156 through 158 of that same file, it compiles the child nodes within the context, meaning both the conditional and the component are compiled with respect to the context.

[dasch]

I'll take a look at fixing this. It's a bit trickier than I had hoped for.

[dasch]

I've been thinking about adding a cd .. block that goes back up the block stack, e.g.

{{@article}}

{{@author}}
  {{name}} posted this on {{@..}}
    {{posted_at}} <!-- `posted_at` refers to the article context here. -->
  {{/..}}.
{{/author}}
{{/article}}

I'm not happy about the syntax, but the semantics would allow us to fix this issue by injecting such a block between the conditional block node and its children.

[teliosdev]

Maybe you can refer to the previous context by having a {{..:posted_at}} syntax.

However, with the "macro" :, maybe it would be fruitful to have that compile to a separate block. Since most presenters don't have any code executed on initialization (i.e. no setup blocks or initialize methods), then it may be fruitful to instead only initialize the presenter once for the "macro" references, so that for each "macro" tag the presenter does not have to be reinitialized.

# normally, curly does this:
options_stack << options
presenters << presenter
buffers << buffer
buffer << presenter.user() do |item|
  options = options.merge("user" => item)
  buffer = ActiveSupport::SafeBuffer.new
  presenter = ::UsersPresenter::ShowPresenter::UserPresenter.new(self, options)
  buffer.concat(presenter.name() { |*args| yield(*args) }.to_s)
  buffer
end

buffer = buffers.pop
presenter = presenters.pop
options = options_stack.pop

# However, since we're probably going to use that presenter again for similar
# things, it makes sense to cache it.
contexts[:user] ||= do
  presenter.user() do |item|
    options = options.merge("user" => item)
    ::UsersPresenter::ShowPresenter::UserPresenter.new(self, options)
  end
end

buffer.concat(contexts[:user].name() { |*args| yield(*args) }.to_s)

This should probably speed things up and use less memory.

[dasch]

I've previously thought about caching presenter instances, however this seems to be orthogonal to the issue at hand... Probably the right thing to do here is to rewrite parts of the compiler to generate different code paths for blocks that change context vs. blocks that don't. I'm not sure when I'll have time for this work, though :-/