Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Cannot access referenced children elements in observe calls. #116

Closed
rubixibuc opened this issue May 18, 2020 · 4 comments
Closed

Cannot access referenced children elements in observe calls. #116

rubixibuc opened this issue May 18, 2020 · 4 comments
Labels
bug render Applies to render and content properties defined as functions

Comments

@rubixibuc
Copy link

rubixibuc commented May 18, 2020

So I've asked about accessing refs before, but this is an interesting use case.

I have an .open boolean prop that when is set to true, I want to focus the input element that is being opened.

Inside of the observe handler for .open, I reference the input element using the ref example provided in the documentation.

The problem is, I'm passing .open into a child element which sets a prop on that child element.

Since the observe callbacks happen in sequence with the get calls it throws an error. Basically "that I'm setting during a get"

Here is a stackblitz with a contrived example, but demonstrates the real problem I'm having.

If I don't set a child element prop base on .open I can reference the input element in the observe handler just fine. But should that be a limitation, and if not, can all observe callbacks be handled after all get's have been settled and resolved?

Other than using references inside observe callbacks, I'm not entirely sure how accessing references would work. Once you use a reference directly or indirectly inside a render, or similar to how I'm using it here it throws errors.

Ty again :)

@smalluban smalluban added bug render Applies to render and content properties defined as functions labels May 20, 2020
@smalluban
Copy link
Contributor

smalluban commented May 20, 2020

Hi @rubixibuc. Thanks for creating this issue. I checked out your example, and I think I didn't take into account some of the cases for the ref idea, which may throw errors. The problem is not related to the observe method directly. Actually, it is called outside of any get call. The error is thrown within getting host.ref. Why? When your component is connected, the observe method from the prop is called before render observe method - so it is called for the first time. Inside of the render, the template sets the false value of the nested component, so the setter is called inside of the get call...

If the render is called when the state of the view is stable, the sequential calls to render won't change anything (there is no change), so there is no set call, and everything works as expected. However, it is quite simple to create a sequence, which throws an error. For example, you can create a button, with callback attached like this:

function handleClick(host) {
  host.prop = !host.prop;
  host.ref;
}

The above code throws the same error as you discovered. The prop value changes, so calling host.ref will force host.render() to update the value of the nested component.

As a result, I have to rethink if that kind of idea has any sense after all. I'm not a fan of direct imperative access to template elements, but I understand a need for that in some cases. One possible solution is deferring update of the template, when called directly (possibly in microtask, by Promise API). However, I am not sure if it does not change an order of updates in a normal process. What do you think?

@rubixibuc
Copy link
Author

rubixibuc commented May 20, 2020

So I'm actually thinking about it a little bit differently

I ran through several alternate implementations in my head and none of them felt as natural as the current one does. I think while rendering can be generally thought of as an asynchronous process, I'm not sure returning the target inside a promise feels natural either, if I'm understanding correctly. I feel like it can lead to subtle bugs by making it asynchronous as a sort of workaround

I'm actually trying to understand why gets and sets are mutually exclusive across different components. If they weren't the current implementation would still work, and require no major library api changes. If the child component was a webcomponent from a library, built with something else, then even though I'm passing it, it wouldn't fail.

Could this behavior be modified, or is it necessary to how the cacheing system works?

The only other viable alternative I came up with, but still doesn't feel great, is an “connected” custom event that can assign event.target(s) to props.

onconnected=${html.setRef('myRef')}

@smalluban
Copy link
Contributor

The root cause behind throwing an error for setting values inside of the get call was to avoid an endless loop of the update process. I assumed that setting the value will notify the parent about the change, which again will update and set the nested value, etc.. The guard was there from the beginning, but the update process, especially the emitter and render factory have changed over time. I did a test - I turned off the guard and tried to break it, but I couldn't ;)

The update of the property in the cycle is done only once, and it can't invoke itself from the get call of the property. After all, I think that it should be safe enough to remove it from the code. There is still a guard that detects cycles, so you can't access the same property inside of the nested getters.

I pushed a fix, which removes the guard. The ref will work properly with the next version for your example.

@smalluban
Copy link
Contributor

The fix released within v4.2.1.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug render Applies to render and content properties defined as functions
Development

No branches or pull requests

2 participants