Skip to content

'simple' resolvers should somehow warn in mustachio if a parent node resolves for them #2667

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

Closed
jcollins-g opened this issue Jun 3, 2021 · 3 comments · Fixed by #2670
Closed
Labels
P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@jcollins-g
Copy link
Contributor

Somehow, it would be nice if a simple resolver would warn if it could have resolved something if it was part of the visibleTypes, but failed to do so. An example case that made me scratch my head for an hour or two was:

<li>{{{linkedName}}}</li>

I had erroneously thought my refactor in #2665 meant I could delete DefinedElementType from the visibleTypes list. When I did so and mustachio went to resolve linkedName here, it skipped over the DefinedElementType inside publicSuperChainReversed and instead resolved linkedName against the class. This meant that the class's name repeated in the superchain list.

This was particularly troublesome to debug as unless someone was actually looking at the generated HTML there was no way to know, and everything looked correct in the debugger, the superchain list objects were correct, and so forth. In fact, if it wasn't for that I remembered I had removed that and noticed that the superchain was a collection of DefinedElementTypes I might still be scratching my head.

I am not sure how to make this warning happen without making simple resolvers much less simple...

@jcollins-g jcollins-g changed the title simple resolvers should somehow warn in mustachio if a parent node resolves for them 'simple' resolvers should somehow warn in mustachio if a parent node resolves for them Jun 3, 2021
@jcollins-g jcollins-g added P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug P2 A bug or feature request we're likely to work on and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Jun 3, 2021
@jcollins-g
Copy link
Contributor Author

Marking as a P2 enhancement, but I am concerned that innocuous refactors that add a new visibleType requirement might cause the templates to resolve against the wrong thing. If this seems to happen a lot, elevate to P1.

@srawlins
Copy link
Member

srawlins commented Jun 4, 2021

Oh my gosh I grok the issue now. What a bummer. I really don't like mustache's resolution rules sometimes (most of the time).

Do you think the solution would be to always error in this case? That is, given the following context object stack:

  • Foo, rendered using a SimpleRenderer, with a field f1 and
  • Bar, rendered using a full renderer, also with a field f1, and a field, Foo foo,

then when rendering a Bar in {{ #foo }} {{ f1 }} {{ /foo }}, we should always error saying

f1 could have been rendered on Foo, but its simple, so now you've put me in the position of skipping over Foo.f1 in favor of Bar.f1. If you are trying to access Foo.f1, please make Foo visible. If you are trying to access Bar.f1, then create a getter with a different name in Bar which doesn't conflict with getters higher up in the stack.

@jcollins-g
Copy link
Contributor Author

Oh my gosh I grok the issue now. What a bummer. I really don't like mustache's resolution rules sometimes (most of the time).

Do you think the solution would be to always error in this case? That is, given the following context object stack:

  • Foo, rendered using a SimpleRenderer, with a field f1 and
  • Bar, rendered using a full renderer, also with a field f1, and a field, Foo foo,

then when rendering a Bar in {{ #foo }} {{ f1 }} {{ /foo }}, we should always error saying

f1 could have been rendered on Foo, but its simple, so now you've put me in the position of skipping over Foo.f1 in favor of Bar.f1. If you are trying to access Foo.f1, please make Foo visible. If you are trying to access Bar.f1, then create a getter with a different name in Bar which doesn't conflict with getters higher up in the stack.

Yes, this, exactly. I think (assuming I understand mustache correctly...) the sticky wicket here for mustache-compliance will be if you do {{ #foo }} {{ f2 }} {{ /foo}} while rendering a Bar, and Bar has f2 but Foo doesn't, to go ahead and allow f2 to be rendered against Bar while preserving the f1 warning behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants