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

[breaking] Stop running in the next loop #232

Closed
wants to merge 1 commit into from

Conversation

jrjohnson
Copy link
Collaborator

This moves back to setting the property synchronously and adds
instructions for handling errors which are caused by this. In this case
causing the error seems better than creating difficult to see timing
bugs when Run.next() is invoked.

Lots of context and discussion and fixes #203.

Context for the original change (reverted here) in #195

This moves back to setting the property synchronously and adds
instructions for handling errors which are caused by this. In this case
causing the error seems better than creating difficult to see timing
bugs when Run.next() is invoked.
@jrjohnson jrjohnson changed the title Stop running in the next loop [breaking] Stop running in the next loop Mar 11, 2020
@jrjohnson jrjohnson marked this pull request as ready for review March 11, 2020 19:04
@jrjohnson
Copy link
Collaborator Author

@lifeart I thought I would try and save you some time by taking a pass at this as a first draft. Let me know if I'm miss-interpreting the conversation over in #203 and I can revise.

@lifeart
Copy link
Owner

lifeart commented Mar 23, 2020

@jrjohnson thank you for PR! I was thinking, glimmerjs/glimmer-vm#1061 may resolve original issue.

I was thinking about @Tracked, we can resolve property descriptor, and if descriptor has "set" method, run action in next runloop, if not - in current

@jrjohnson
Copy link
Collaborator Author

OK, shall I close this an await a glimmer solution? We can work around this in the interim by setting the values we need using {{did-insert}} manually.

@lifeart
Copy link
Owner

lifeart commented Mar 24, 2020

@jrjohnson what do you think about sync/async logic, based on property descriptor?
tldr: if property defined and has getter/setter - use async implementation, if property not defined or has "simple" descriptor - use sync implementation

@jrjohnson
Copy link
Collaborator Author

Sorry @lifeart, I don't have enough understanding of the underlying tech to comment either way, I'm happy to test it out though!

@lifeart
Copy link
Owner

lifeart commented Oct 8, 2020

Since this addon counted as legacy, closing this merge request in favour of https://github.com/lifeart/ember-ref-bucket
@jrjohnson thank you for your work on it and pushing this case forward! Hope updated implementation bring more clarity and stability.

@lifeart lifeart closed this Oct 8, 2020
@jrjohnson jrjohnson deleted the not-next branch October 8, 2020 15:58
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.

check interaction with render modifiers
2 participants