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

Overriding compile to support other template formats #1547

Open
AndrewLeedham opened this issue Sep 9, 2019 · 4 comments
Open

Overriding compile to support other template formats #1547

AndrewLeedham opened this issue Sep 9, 2019 · 4 comments

Comments

@AndrewLeedham
Copy link
Contributor

As per #1534:

The following JS Fiddle shows how we are overriding the handlebars compile function in order to allow mustache templates to be optionally used: https://jsfiddle.net/AndrewLeedham/1Ljzkneh/28/

So essentially it is overriding compile and checking if a partial has its handler set to mustache exists given the name of the current template being rendered. If so it uses Mustache.render, otherwise it carries on with the normal Handlebars compile. All the partials have to be registered to handlebars, otherwise the callback returned from Handlebars.compile is never run.

@nknapp
Copy link
Collaborator

nknapp commented Sep 9, 2019

I have to think about this. This code looks hacky to me, but I am not sure about the alternatives.

Update: Please don't be offended by the work "hacky". What I mean is: The way Handlebars is used here is not documented and only possible because JavaScript lacks mechanisms of prohibiting certain kinds of use.

As developer who uses open source libraries, I can understand why you are doing this: You need a solution now, and it is often easier and faster to work around missing features in the library than to submit a PR and hope that the maintainer will merge it eventually.

As maintainer, I have to say: The workaround is a hack, because this kind of use was not intended by the library author (or it would be documented). This kind of use makes it hard to maintain a library like this. I cannot anticipate all the use-cases that user of my library comes up with. This means that while fixing bugs, adding new features or during refactorings, things might break for that user.

What else could you do?

You could register a "compiled" partial like in https://jsfiddle.net/hq413g0a/, but this is equally undocumented and there is no test in the spec that ensures this behaviour. It just works, because the partial is replaced by its compiled function in the partials-array. I'm thinking about adding a test to the spec to make this official.

From the current API, It would be cleaner to register a helper for the mustache partials, but I can see that this would require a lot of adaption in your code.

@AndrewLeedham
Copy link
Contributor Author

I agree this is hacky. We plan to move away from the pattern eventually, but as you say it is a solution that works now. Just thought I'd share it so you are aware of how handlebars may be used in some cases, so you can use that information for future changes.

Not sure what you referring to with a "compiled" partial, is that essentially registering a partial as a function?

Yes a helper wouldn't be ideal in our use case.

@nknapp
Copy link
Collaborator

nknapp commented Sep 10, 2019

You usually register a string as partial, but when you include the partial for the first time, it is replaced by the compiled function. What I have done is registering a function directly. Something like that also happens when you precompile partials using the handlebars binary.

https://jsfiddle.net/xrdonyc5/2/

@nknapp
Copy link
Collaborator

nknapp commented Sep 20, 2019

@wycats I would like to know if you would consider registering a function as a partial is considered part of the API. It is possible at the moment, but undocumented.

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

No branches or pull requests

2 participants