emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.46k stars 4.21k forks source link

Streams and HTMLBars helpers are not accessible to developers #10150

Closed tim-evans closed 9 years ago

tim-evans commented 9 years ago

I'm working on migrating our app from 1.8 to 1.10.0-beta.3 and I'm encountering difficulties in trying to slowly simplify helpers that I've written. The switch to HTMLBars is causing some pain points here.

I have a few helpers that disobey the assertion when creating a helper that it must have a dash. I got away with this by using Ember.Handlebars.registerHelper.

I'm finding myself unwrapping the Ember.Handlebars.registerHelper call so it returns the original parameters that HTMLBars helpers take. Could we expose these helper functions? (As well as stream helpers?)

I understand that this is a low layer of the API; it's just frustrating to have your helpers no longer work and then find your tool belt is missing several important tools.

workmanw commented 9 years ago

Ditto. I too was trying to migrate a helper today and found it very hard to work with (and debug) streams with most of it wrapped up in closures. A little documentation on view.getStream might go a long way.

mmun commented 9 years ago

See https://github.com/emberjs/ember.js/pull/9693.

@tim-evans @workmanw Can you explain the types of helpers that require manually working with streams to migrate?

workmanw commented 9 years ago

I have two helper use cases.

1) Deals with looking up a custom action targets. We have components that need to be able to target a parent view or other target, this is currently supported because Ember.Component.targetObject doesn't allow it. So we provide our own target arg and look it up via a stream.

2) Our basic input components (input/textarea, select, checkbox, button, etc) all use a custom view mixin ("ErrorMixin"). The TL;DR; is that these components setup a custom stream dynamically when they're value is bound to an ember data model to handle validation errors. So on('init') we lookup the valueBinding and setup a secondary dynamic binding to the errors hash. E.g. if valueBinding="user.age" we setup a stream to keep error in sync with user.errors.age.

With regards to 2), I'm sure I'm going to catch loads of flack for this not being "Ember Proper". But EDs validation handling (DS.Errors) creates a lot of boiler plate and this was our attempt at reducing it.

tim-evans commented 9 years ago

We have a custom translation helper that is named t (and a localization helper named l), which binds to a interpolated string:

{{t "invoice.`status`"}}

Where the string ends up dynamically being created using bindings. I would like to decompose our helpers to string concatenation and translation. This means I need to write a join helper that returns a stream.

{{t (join "invoice" status)}}

I don't currently see any solution for providing dynamic concatenation of strings that are passed into a helper, so this is where I'm left.

The other use cases involve shorthand action helpers that trigger events on the router / controller. (They are popup-dialog and a (as in 'action')).

I would prefer keeping some of our helpers terse, since they are used all over our app. Having a helper creation function to bypass the - requirement of components is important for this use case (especially if we're using localization and internationalization for HTML attributes)

mmun commented 9 years ago

@tim-evans That's great, thanks. You may also be interested in writing an HTMLBars AST transform that rewrites {{t "invoice.status"}} to {{t (join "invoice" status)}} at compile time. It's now possible to write ember addons that do this! :)

@workmanw Sorry I'm having a hard time understanding. Could you show me an example of how you'd use your helper in a template?

tim-evans commented 9 years ago

@mmun That would be a fantastic way to not change all that code. Thanks for the idea!

mmun commented 9 years ago

https://github.com/emberjs/ember.js/pull/9693 was merged, we just need to "go" it at the next weekly meeting.

stefanpenner commented 9 years ago

pending glimmer, as it may slightly change some stream API's

mmun commented 9 years ago

With glimmer we think won't need to expose streams.

tim-evans commented 9 years ago

@stefanpenner thanks for the update :+1: The new APIs are much simpler.

@mmun I'm not sure about that statement. I'm currently creating streams for a feature-flag helper. If you can figure out a way to not expose streams to create this LMK.

https://gist.github.com/tim-evans/e8f9f65e9b7dfc90a335

mmun commented 9 years ago

@tim-evans Seems like you could just use a tagless component that plugs into a features service for this?

tim-evans commented 9 years ago

@mmun would a tagless component work in an if statment?

ie.

{{#if (feature-flag "invoices")}}
  Invoices
{{/if}}
mixonic commented 9 years ago

For reference, here is what the new if/else helper looks like: https://github.com/emberjs/ember.js/blob/idempotent-rerender/packages/ember-htmlbars/lib/helpers/if_unless.js#L25

I'm just catching up on this stuff now though, and I'm unsure if you could just observe and call options.template.yield(); at any time or if there is something additional required.

mmun commented 9 years ago

@tim-evans there isn't a public api planned yet to handle that case. In glimmer, helpers will essentially be pure functions and won't manage streams themselves (similar to bound helpers today). There is a concept of "keywords" that are much more powerful. They're used internally to implement many of pre-glimmer's helpers (pretty much all of em). Were keeping them private until we collect enough use cases and iron out the API.

For now this should suffice for most purposes:

{{#feature "invoices}}
  ...

Please excuse typos. Typing on my phone.

stefanpenner commented 9 years ago

@mmun do we consider this a bug, or is this internal API now less leaky?

mixonic commented 9 years ago

IMO this has been addressed. Please re-open if I've missed anything.

The new helpers API, and nested helper usage, seems to cover the use-cases discussed without exposing streams. We could still improve by offering an interpolation helper out-of-the-box with Ember. Streams may eventually become public, but that discussion should be in an issue or RFC on https://github.com/emberjs/rfcs

tim-evans commented 9 years ago

Everything should be good with the new helper API. Thanks for the hard work!