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

Deprecate decorators #1574

Open
nknapp opened this issue Oct 9, 2019 · 10 comments
Open

Deprecate decorators #1574

nknapp opened this issue Oct 9, 2019 · 10 comments

Comments

@nknapp
Copy link
Collaborator

nknapp commented Oct 9, 2019

The decorator API currently exposes a good portion of internal Handlebars methods (like the container-object) that should not be part of the official API. This makes it difficult to write a formal specification and port this API to implementations in other programming languages.

In the next minor version of Handlebars, the API will be deprecated and warning messages will be written to the console, when attempting to register a decorator.

If you use decorators, please add a comment here so we can discuss alternatives.

@allouis
Copy link

allouis commented Oct 19, 2019

Out of interest, would only the registering of custom decorators be deprecated, or would support for all of them eventually be deprecated/removed?

I'm specifically interested in the usage of the inline decorator inside of partial blocks - like the example from the current docs:

{{#> layout}}
  {{#*inline "nav"}}
    My Nav
  {{/inline}}
  {{#*inline "content"}}
    My Content
  {{/inline}}
{{/layout}}

@nknapp
Copy link
Collaborator Author

nknapp commented Oct 19, 2019

Only registering custom decorators will be deprecated. I have discussed this and other matters with Yehuda and we won't even remove this in 5.0, more like 6.0

And I don't think 5.0 is going to come this year.

But I still need to know if anybody object, because if somebody does, I would like to find a solution. We may offer a similar API. My only problem with the current implementation is the amount of exposure of private code.

nknapp added a commit that referenced this issue Oct 28, 2019
the function "registerDecorator" is now deprecated. The
decorator API exposes a lot of internal structures that
should not be part of the official API. An alternative
API can be discussed at #1574
- "Handlebars.registerDecorator" now logs a warning
- "Handlebars.registerDecoratorWithoutDeprecationWarning"
  can be used in order to omit the warning, when
  developers have read the notice.
  (The inline-decorator is using this function now)
@cthorner
Copy link

Is there an alternative to decorators? Or just use helpers more extensively? Seems like this is going to add some complexity to templates already using decorators.

@nknapp
Copy link
Collaborator Author

nknapp commented Oct 30, 2019

@cthorner Thank you for joining the discussing. If you need decorators, what parts of the API you are using?

I am bothered mostly about the container being exposed. If you are using any function from the container I would like to know which ones and for which purposes. In any case, I would be interested in example decorators, because all that I have seen so far is the inline-decorator and a minimal textbook-example from sitepoint as well as in the test-cases. Apart from inline I have no idea what kind of decorators are used in production anywhere. If you can't publish your code, you can also send me an email.

We can develop alternatives. But I need to know the use-cases.

@nknapp nknapp added this to the 5.0.0 milestone Apr 5, 2020
@jbboehr
Copy link
Contributor

jbboehr commented Apr 16, 2020

(slightly off-topic) I haven't implemented decorators in my port/implementation (handlebars.c) in part due to this, mostly due to it being really hairy in C, but also:

Every single opcode works fine in a normal interpreter as-is except registerDecorator. It basically needs to be executed in it's own context right before the current block. It's not a huge deal, but would be nice to see improved.

I'm a bit out-of-date (4.0.5) so if things have changed since then, I apologize.

{{foo}}
{{* activateFormatter}}
{{formatMoneyHelper this.start}}
getContext[LONG:0]
pushProgram[NULL]
pushProgram[NULL]
getContext[LONG:0]
lookupOnContext[ARRAY:foo][NULL][BOOLEAN:1][BOOLEAN:0]
invokeAmbiguous[STRING:foo][BOOLEAN:0]
appendEscaped
appendContent[STRING:\n]
pushProgram[NULL] <-- here
pushProgram[NULL]
emptyHash[NULL]
registerDecorator[LONG:0][STRING:activateFormatter] <-- to here
appendContent[STRING:\n]
getContext[LONG:0]
lookupOnContext[ARRAY:start][NULL][NULL][BOOLEAN:1]
pushProgram[NULL]
pushProgram[NULL]
emptyHash[NULL]
getContext[LONG:0]
lookupOnContext[ARRAY:formatMoneyHelper][BOOLEAN:1][BOOLEAN:1][BOOLEAN:0]
invokeHelper[LONG:1][STRING:formatMoneyHelper][BOOLEAN:1]
appendEscaped
appendContent[STRING:\n]
-----

@nknapp
Copy link
Collaborator Author

nknapp commented Apr 18, 2020

I take that as a vote for deprecating and replacing the current API. So far I haven't heard a single vote keeping it.

My guess is that nobody really uses custom decorators, but plenty of people use inline partials.

But since you joined the discussion: do you have an idea how to make better API for registering decorators? On that would allow you to implement it in handlebars.c and that would allow to implement the *inline decorator as well?

@jbboehr
Copy link
Contributor

jbboehr commented Apr 19, 2020

Well, having not thought about it a lot, if it were up to me, I would probably just implement inline partials as an operator ({{<name}}) and remove the whole thing.

The tricky part in C is dealing with multiple nested closures - not exactly what C excels at :) But that's kind of what a decorator is AFAICT.

Also, well, my implementations are meant to be used with helper/partial loaders [0] [1] so I can't really create a partial stack frame like in the inline helper, at least without significant changes to how the loaders work. Not to mention I have to deal with converting back and forth between C and PHP.

This is probably totally unsuitable for your implementation, but I guess it would be easier to implement if the decorator returned a list of partials and helpers (and decorators? 💀) it wanted registered for the block, say:

registerDecorator('inline', function(fn, options) {
  let partials = {};
  partials[options.args[0]] = options.fn;
  return {
    partials,
  };
});

the return type could be something like (sorry my typescript is rusty)

interface DecoratorRetval {
   helpers: object | null;
   partials: object | null;
   fn: null | (any, any) => any;
}

and the runtime would transparently handle snapshotting and merging the objects at the appropriate time. TBH, in my implementation I'd probably implement it by mutating the helper/etc objects and keeping a copy of the keys that are changed.

@sunng87
Copy link

sunng87 commented Apr 19, 2020

BTW, is there an official definition for decorators?

In my rust implementation, decorators are allowed to

  • Register render scoped helpers and partials
  • Mutate context object in a copy-on-write way

I had an feature request from my users to use decorators: sunng87/handlebars-rust#318

@nknapp
Copy link
Collaborator Author

nknapp commented Apr 19, 2020

@jbboehr thanks for your detailed thought. They are certainly helpful. I don't know when we get around to work on this, but its good to have some ideas. What I like about your approach is that it becomes clear what a decorator is supposed to modify, instead of just passing the whole container.

Although, I would keep the signature like the ones for helpers (at the moment):

registerDecorator('inline', function(name, options) {
  let partials = {};
  partials[name] = options.fn;
  return {
    partials,
  };
});

If you have problems with nested closures, then partial-blocks are a problem too, aren't they?

@nknapp
Copy link
Collaborator Author

nknapp commented Apr 19, 2020

@sunng87 Decorators where there when I took over maintenance. I wasn't there when the idea was discussed, but I have seen the following:

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

5 participants