Skip to content
This repository has been archived by the owner on Nov 25, 2020. It is now read-only.

Fix component promise #61

Closed
wants to merge 1 commit into from

Conversation

doktordirk
Copy link
Contributor

wip

@RWOverdijk
Copy link
Member

What am I looking at? Why is it wip? Also, why weren't you taking your break like you said you would :p ?

@doktordirk
Copy link
Contributor Author

hehe
first the component test source: https://github.com/aurelia/testing
second promise issue, there it see there aurelia/framework#367 (comment):

@doktordirk
Copy link
Contributor Author

i separated the two.test PR wip #63

@doktordirk
Copy link
Contributor Author

note to self: see if async/await isn't easier

ps: user'd need to install babell-polyfills. so, not the best option.

@RWOverdijk
Copy link
Member

@doktordirk Why the extra promise? API already returns a promise.

@doktordirk
Copy link
Contributor Author

had a reason 😄 need to have a look

@doktordirk
Copy link
Contributor Author

i remember now. there can be errors thrown in the top parts which i wanted to be able to handle in a .catch()

(managed to get errors thrown in my first trails for tests)


return repository.findPath(findPath, criteria);
resolve(repository.findPath(findPath, criteria));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return in stead of resolve

@RWOverdijk
Copy link
Member

Could you explain this PR to me again please? I still don't quite understand.

@doktordirk
Copy link
Contributor Author

first, i mainly did it to promises in components. thus it's mainly for your consideration. i wouldn't insist as there might be other options, or a no-problem in the first place

components have no activate() and thus don't respect promises. so, when attached is called the promise coming from fetch might not be resolved or rejected yet. as described.in the links compositionTransaction is a way to make attached wait until the fetch promise is fulfilled

@RWOverdijk
Copy link
Member

@doktordirk What you're describing is by design. You never need the data to be populated instantly. Now, if you have 4 association-selects on the page, you have to wait, which is a bad UX. That's the reason it's in activate().

@doktordirk
Copy link
Contributor Author

yeah, you have those observe something in there. i better have another look at the flow

@RWOverdijk
Copy link
Member

@doktordirk If you hit me up on gitter when you have time I can give you the "gist"

@doktordirk
Copy link
Contributor Author

i'll treat it as excercise

@doktordirk doktordirk closed this Apr 2, 2016
@doktordirk doktordirk deleted the fix-component-promise branch September 8, 2016 13:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants