emberjs / ember.js

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

input and textarea template helpers throw when yielded #12690

Closed makepanic closed 8 years ago

makepanic commented 8 years ago

Example twiddle with components that yield a link-to component, input helper and textarea helper: https://ember-twiddle.com/e175f5ad0245fc897cc8?numColumns=1 (Click the button to run the code to see the error inside your js console.)

The link-to, yielded using the component helper and concat work without throwing.

{{yield (hash val=(input value='inputValue'))}}

throws: Uncaught TypeError: keyword is not a function

{{yield (hash val=(textarea value='20'))}}

throws: Uncaught TypeError: Cannot read property 'getState' of null

pixelhandler commented 8 years ago

@makepanic are input and textarea helpers expected to work when yielded in this fashion? I see in your example that the other components x-concat and x-link (using the concat and component helpers) do work.

I have not use this canary feature yet for yielding via the hash. Just looking at this example makes me think there may be a less terse way to use the yield + hash.

My expectation of the yield with hash is that it's a good way to expose an interface (vs yield this). using the helpers inside the val seems a bit too clever in my opinion, personally I expect to see those helpers used plainly in an .hbs template rather then used so tersely. But I'm not up to speed with the design of the hash helper used with the yield function of a component.

I'm curious, do you have any links that support this use case for the textarea and the input helpers?

rwjblue commented 8 years ago

Yeah, this should likely work (via the new contextual components feature).

@serabe - This might be another one for you if you have time. If not, I can try to tackle sometime next week.

Serabe commented 8 years ago

The example as is should not work: it needs to use the component helper:

{{yield (hash val=(component "input" value='inputValue'))}}
{{yield (hash val=(component "textarea" value='20'))}}

That said, those example does not work either since the element component helper (the one that renders a component given its name) does not work with them either. I can add a whitelist here maybe tomorrow morning (currently is 2:52am) or on Sunday if you think is appropriate.

Serabe commented 8 years ago

Not as simple as I thought. The thing is that input and textarea are keywords, not components. The components are -text-field, -checkbox and -text-area and those seem to work.

I think I can refactor those keywords to behave like component, but could seem unfair that what seems like a built-in component has better syntax for contextual components. Maybe something like the old makeViewHelper but for contextual components?

Let's discuss this on Slack when we meet, please.

/cc @mixonic

mixonic commented 8 years ago

I believe one reason for not having these as public component APIs was a fear that they would conflict with the glimmer versions of these same things.

rwjblue commented 8 years ago

@mixonic - It should be possible to yield a hash containing an input or textarea. Do you disagree?

mixonic commented 8 years ago

I agree. I'm less certain about making {{component 'input'}} or (component 'input') (for example) work with non-glimmer components. Definitely not crazy about (input as a non-glimmer component.

Serabe commented 8 years ago

I created a PR with a tentative implementation for this feature so you can play with it a bit.

makepanic commented 8 years ago

I added your branch to my ember project and from what i can tell, it works as expected. :+1:

novaugust commented 8 years ago

The docs suggest that you can currently use "input" with the component helper (https://github.com/emberjs/ember.js/blob/v2.3.1/packages/ember-htmlbars/lib/keywords/component.js#L59-L70) Might be worth updating them until a fix for this is in?

Serabe commented 8 years ago

Done https://github.com/emberjs/ember.js/pull/13010

locks commented 8 years ago

@Serabe @mixonic @rwjblue do we have resolution on this issue?

Serabe commented 8 years ago

@locks: AFAIK input and textarea cannot be called with the component helper (they are not even components per se). I wrote a solution in my blog. Further discussion was delayed until the new components are done (if my memory does not fail me, which usually does).

makepanic commented 8 years ago

The workaround described in http://www.serabe.com/2016/03/02/the-path-to-contextual-components-understanding-dynamic-components/ works without problems and solves this issue.