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 names #828

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Dynamic names #828

wants to merge 5 commits into from

Conversation

lucas42
Copy link

@lucas42 lucas42 commented May 13, 2024

The mustache spec now includes an optional part of the spec for using dynamic names in partials: https://github.com/mustache/spec/blob/master/specs/~dynamic-names.yml

I've not contributed to any mustache projects before, so I've tried to keep my code change as concise and self-contained as possible, whilst getting all the relevant tests in the spec to pass.
(If you'd prefer something more verbose to aid code readability, I'm happy to adjust)

Skip new interpolation test and optional inheritance part of the spec
Node 0.10 and 0.12 don't implement the `startsWith` function on strings
@lucas42
Copy link
Author

lucas42 commented May 31, 2024

There's a problem with this change, which none of the tests have flagged. Whilst it works the first time for rendering a given dynamic partial, the way the caching works is that'll it'll continue to render that same partial for subsequent calls, even if a different name is passed in.

@lucas42
Copy link
Author

lucas42 commented Jun 1, 2024

Okay, I've managed to fix the caching problem. Have added a test to verify that.
Though it does make this change more complicated than originally envisaged. Would appreciate any feedback as I'm a bit out of my depth with the parsing logic.

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 this pull request may close these issues.

1 participant