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

Dynamic Partials #96

Closed
amaclean opened this issue Feb 10, 2016 · 6 comments
Closed

Dynamic Partials #96

amaclean opened this issue Feb 10, 2016 · 6 comments

Comments

@amaclean
Copy link
Contributor

@amaclean amaclean commented Feb 10, 2016

Hey Rex, I'd like to have a go at adding support for dynamic partials but I wanted to get your take on it first. Despite the existing support for subexpressions I have a feeling that may be a relatively invasive change. Do you have a preferred approach to take?

@rexm
Copy link
Collaborator

@rexm rexm commented Feb 10, 2016

Haven't used this feature. Does it pass any arguments to the function?

@amaclean
Copy link
Contributor Author

@amaclean amaclean commented Feb 11, 2016

Yeah, it seems to support the same functionality as subexpressions passed to helpers. Only in this case it replaces the partial name and the string returned is used to find the partial instead.

I'm guessing I'd need to change the way the partial token is parsed so that it can convert the subexpression and its arguments, before continuing to grab any arguments for the partial as normal.

This would need to be stored in the partialexpression somewhere, could change the type of the name property to an expression if that could also still represent a string somehow. Handlebars js does if this way and has an isDynamic property to determine how it's handled in invokePartial. Obviously that's easier for a loosely typed language so it may need to be done differently here.

Then it would be a case of adding a visit method for partial expressions in the appropriate places (HelperFunctionBinder and SubExpressionVisitor at a guess).

I haven't thought about how to get the string returned from the helper to the InvokePartial method for the partial name parameter yet.

Let me know if that makes sense or if I'm just rambling!

@rexm
Copy link
Collaborator

@rexm rexm commented Feb 11, 2016

Most of that doesn't actually sound too bad... Modifying the Partial to be able to have a child Expression property which is accumulated the same way any other expression would be sounds like a minor change. We already have cases where words are resolved down to ConstantExpressions, a simple partial statement could follow that and anything more complex would follow existing SubExpr logic, like you said.

Ultimately the > token would need to represent the partial expression with an input Expression that resolves to a string value. Not bad at all...

And I guess just need to add a member type check for method to the PathBinder, because as I understand it, the function in question is actually on the model, not a registered helper, right?

@amaclean
Copy link
Contributor Author

@amaclean amaclean commented Feb 19, 2016

So I finally found some time to take a look at this and as you said, the first part was easy enough. I'm now at the point where we have an Expression for the name in the VisitPartialExpression method and if this is ConstantExpression then all is well and everything works normally (all unit tests pass).

What I'm struggling to figure out is when this is a SubExpression how I get it resolved down so that it can be passed to the InvokePartial method. I've experimented with a couple of new Visit methods on the PartialBinder but with no success, I've not quite got my head around how this part hangs together yet. Do you have any suggestions?

Here is what I currently have.

@rexm
Copy link
Collaborator

@rexm rexm commented Feb 20, 2016

It seems like you're quite close. I pulled down your latest and added the following test:

[Test]
public void SubExpressionPartial()
{
    string source = "Hello, {{> (partialNameHelper)}}!";
    Handlebars.RegisterHelper("partialNameHelper", (writer, context, args) =>
            {
                writer.Write("partialName");
            });
    using (var reader = new StringReader("world"))
     {
        var partial = Handlebars.Compile(reader);
        Handlebars.RegisterTemplate("partialName", partial);
    }
    var template = Handlebars.Compile(source);
    var data = new {};
    var result = template(data);
    Assert.AreEqual("Hello, world!", result);
}

I made this test pass by changing line 42 of PartialBinder.cs to:
Expression.Convert(pex.PartialName, typeof(string)),

See how this affects your tests.

@amaclean
Copy link
Contributor Author

@amaclean amaclean commented Feb 20, 2016

Ha, that did the trick, I can't believe it was that simple! I even looked at Expression.Convert() but I was sure there was something else that needed to happen first. Thanks for the help, I'll get a PR ready shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants