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

Stache promise support doesn't work with ES6 promises #2359

Closed
nlundquist opened this Issue Apr 5, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@nlundquist
Collaborator

nlundquist commented Apr 5, 2016

https://canjs.com/docs/can.stache.html#section_WorkingwithPromises

The support described here for promises in stache templates appears to only apply to jQuery deferreds. ES6 promises are just treated as any other object in a template.

If this is intended the docs should be updated to reflect it.

A test case to demonstrate: https://github.com/nlundquist/canjs-template-promise-test

Correction: this is resolved by #2247

@matthewp

This comment has been minimized.

Show comment
Hide comment
@matthewp

matthewp Apr 5, 2016

Contributor

Looking at your example, I don't think native Promises have a value property, could this be why?

Contributor

matthewp commented Apr 5, 2016

Looking at your example, I don't think native Promises have a value property, could this be why?

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Apr 5, 2016

Contributor

That's not why. We add one.

Sent from my iPhone

On Apr 5, 2016, at 6:59 AM, Matthew Phillips notifications@github.com wrote:

Looking at your example, I don't think native Promises have a value property, could this be why?


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub

Contributor

justinbmeyer commented Apr 5, 2016

That's not why. We add one.

Sent from my iPhone

On Apr 5, 2016, at 6:59 AM, Matthew Phillips notifications@github.com wrote:

Looking at your example, I don't think native Promises have a value property, could this be why?


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub

@nlundquist

This comment has been minimized.

Show comment
Hide comment
@nlundquist

nlundquist Apr 5, 2016

Collaborator

I think we should add the same properties to native promises. Or at least document that this support is only for deferreds.

Collaborator

nlundquist commented Apr 5, 2016

I think we should add the same properties to native promises. Or at least document that this support is only for deferreds.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Apr 5, 2016

Contributor

A+ promises must be working in some fashion, otherwise, all the promises produced by can-connect, which are not native, but A+ (which is close enough) do work.

Contributor

justinbmeyer commented Apr 5, 2016

A+ promises must be working in some fashion, otherwise, all the promises produced by can-connect, which are not native, but A+ (which is close enough) do work.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Apr 5, 2016

Contributor

Also, we don't need to add anything to native promises, we only have to make sure can.isPromise works for them. My guess is that StealJS's polyfill is making promise instanceof Promise fail.

Contributor

justinbmeyer commented Apr 5, 2016

Also, we don't need to add anything to native promises, we only have to make sure can.isPromise works for them. My guess is that StealJS's polyfill is making promise instanceof Promise fail.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Apr 5, 2016

Contributor

This shows native promises working if can.isPromise is fixed: http://jsbin.com/xitezi/1/edit?html,js,output

Contributor

justinbmeyer commented Apr 5, 2016

This shows native promises working if can.isPromise is fixed: http://jsbin.com/xitezi/1/edit?html,js,output

@nlundquist

This comment has been minimized.

Show comment
Hide comment
@nlundquist

nlundquist Apr 5, 2016

Collaborator

awesome thanks for looking into this

Collaborator

nlundquist commented Apr 5, 2016

awesome thanks for looking into this

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