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

Inconsistent syntax for partials with dynamic names and context #262

Closed
mickeyreiss opened this issue Apr 17, 2013 · 4 comments · Fixed by #635
Closed

Inconsistent syntax for partials with dynamic names and context #262

mickeyreiss opened this issue Apr 17, 2013 · 4 comments · Fixed by #635

Comments

@mickeyreiss
Copy link

Hey guys, I noticed some inconsistencies in the syntax for invoking partials with dynamic names and a scope.

Let's say this is my scope:

{
  me: { name: Mickey, company: "Medallia" },
  partialName: "person"
}

And this is a partial that I've got saved in person.dust:

Hi, I'm {name}, and you can find me at {company}!

Here's where the problems begin. This is my template:

{#me}
  {> person /}
{/me}
{?me}
  {> person:me /}
{/me}
{?me}
  {> person name=me.name company=me.company /}
{/me}

{! So far so good... !}

{#me}
  {> "{partialName}" /} {! <-- This works !}
{/me}
{?me}
  {> "{partialName}":me /} {! <-- This throws a template "[blank]" not found error !}
{/me}
{?me}
  {> "{partialName}" name=me.name company=me.company /} {! <-- and this one also throws an error :( !}
{/me}

I would expect this to output "Hi, I'm Mickey, and you can find me at Medallia!" six times. However, the last two partial invokations throw errors.

My understanding is that this behavior is inconsistent. See #261 for failing the failing specs.

Do you guys agree? Is there a rationale for the inconsistent syntax?

Thanks!

P.S. My actual use case for this functionality is rendering a heterogeneous collection, in which each entity is aware of its subtemplate and subtemplate-scope. I would like to render the subtemplate, even if the scope is empty, which makes the colon-syntax cleaner than a conditional.

@kate2753
Copy link
Contributor

This issue is same as #383

{> "{partialName}":me /} {! <-- This throws a template "[blank]" not found error !}

Dust is stepping into :me context before it resolves partialName. It tries to find partialName within :me context, which does not have it, so this fails.

Colon syntax limits value lookup only to that context, preventing going up the context chain and into global. So in example above Dust will try to find partialName in me context only and will not look for it anywhere else.

Being limited to only data in the context set with colon syntax is very constraining and un-intuitive. We are considering deprecating this syntax in favor of using parameters as more familiar and predictable workaround:

{>"{partialName}" myData=me/}

inside the partial

{#myData}
  Hi I'm {name} and you can find me at {company}!
{/myData}

@rragan
Copy link
Contributor

rragan commented Dec 18, 2013

I never found the :context useful except to provide access to a second set of context stack values that might otherwise not be reachable. With the 2.0 change to let paths reach up the stack, this use is no longer of interest further eliminating a need for it. What is our deprecation notification mechanism? WIki?

@kate2753
Copy link
Contributor

Ideally we would release a version of Dust that prints warning messages when colon syntax is encountered saying it's deprecated and will be removed in new version. Immediately after that publish a new version on Dust that does not support colon syntax.

We should document this on Wiki. Also having some kind of release notes would be great. We should start it if we don't have it already.

@smfoote has a linting tool for Dust. We can extend it to find colon syntax and make it available here, so migration path is somewhat easier.

This will have to be major version bump since this is backward incompatible. Perhaps we should bundle this with possible whitespace change #238, to minimize number of major releases that require migration.

@sethkinast sethkinast removed this from the Milestone 2.4 - fast follow for 2.3 milestone Mar 19, 2015
sethkinast pushed a commit to sethkinast/dustjs that referenced this issue Apr 17, 2015
When scoping a partial to a context:

    {>"{partial}":context /}

The evaluation of the partial name was done in `context` instead of the current scope.

Fixes linkedin#262
Closes linkedin#261
sethkinast pushed a commit to sethkinast/dustjs that referenced this issue Apr 17, 2015
When scoping a partial with a dynamic name to a context like this:

    {>"{partial}":context /}

The evaluation of the partial name was done in `context` instead of the current scope.

Fixes linkedin#262
Closes linkedin#261
sethkinast pushed a commit to sethkinast/dustjs that referenced this issue Apr 17, 2015
When scoping a partial with a dynamic name to a context like this:

    {>"{partial}":context /}

The evaluation of the partial name was done in `context` instead of the current scope.

Fixes linkedin#262
Closes linkedin#261
@sethkinast
Copy link
Contributor

Took long enough, but it's fixed!

"I'm Mickey, and you can find me at Medallia" has haunted me in my dreams FWIW

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 a pull request may close this issue.

4 participants