Skip to content
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

bug: ref called in specified order and not after initializing element with all its attributes/properties #4074

Closed
3 tasks done
jcfranco opened this issue Feb 17, 2023 · 4 comments · Fixed by #5614
Closed
3 tasks done
Assignees
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@jcfranco
Copy link
Contributor

Prerequisites

Stencil Version

2.20.0/3.0.1

Current Behavior

The order of ref matters and can lead to props/attributes being outdated by the time the callback is invoked.

Expected Behavior

ref is called right after the element is fully initialized with all props/attributes. This aligns with other libraries that implement a similar ref mechanism (e.g., React, preact, maquette).

System Info

OS X 13.2.1, npm 8.3.1, node 16.13.2, Chrome 110.0.5481.100

Steps to Reproduce

Follow the steps to the repro case below.

Code Reproduction URL

https://github.com/jcfranco/stencil-ref-called-in-prop-order-instead-of-after-node-being-initialized

Additional Information

For comparison, I also created the following repro cases:

@ionitron-bot ionitron-bot bot added the triage label Feb 17, 2023
@alicewriteswrongs alicewriteswrongs self-assigned this Feb 17, 2023
@alicewriteswrongs
Copy link
Contributor

alicewriteswrongs commented Feb 17, 2023

Hi @jcfranco, thanks for reporting this issue!

We have a few issues relating to ref and order-of-operations which have been previously reported. For instance #3564 reports issues with new and old refs, and #3253 reports on a situation where a ref can be set to null in a situation where it shouldn't.

Is it possible that this issue is reflective of the same underlying problem? By that I mean, those particular issues really can only be fixed by refactoring how Stencil's virtual DOM works to commit changes in an atomic way. We have plans to scope out and make that sort of fix in the future, but haven't been able to work it in to our plans just yet.

Anywhooo - if you think that this is a separate issue we can for sure keep this issue open, but if it's a different symptom of the same underlying problem it might be good to add some of the information here on one of the other existing issues.

Thanks again for filing and providing a reproduction case! I know that these ref and rendering callback order errors are not great to run into.

@alicewriteswrongs alicewriteswrongs added the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Feb 17, 2023
@ionitron-bot ionitron-bot bot removed the triage label Feb 17, 2023
@jcfranco
Copy link
Contributor Author

@alicewriteswrongs Thanks for reply. 😄

if you think that this is a separate issue we can for sure keep this issue open, but if it's a different symptom of the same underlying problem it might be good to add some of the information here on one of the other existing issues.

I think this could remain open mainly because the referenced issues above have no context about the order of props/attributes affecting the ref callback node. I think that makes it different from the others, but I'll let you make that call.

@ionitron-bot ionitron-bot bot removed the Awaiting Reply This PR or Issue needs a reply from the original reporter. label Feb 23, 2023
@alicewriteswrongs
Copy link
Contributor

Sure, we can leave this open for now. We do have plans to address these ref and lifecycle related issues in a holistic way, so I'll add this issue to a list which we're using to validate any possible changes / check for regressions / etc.

Thanks for filing!

@alicewriteswrongs alicewriteswrongs added the Bug: Validated This PR or Issue is verified to be a bug within Stencil label Mar 1, 2023
jcfranco added a commit to Esri/calcite-design-system that referenced this issue Mar 3, 2023
**Related Issue:** N/A

## Summary

This updates `ref` usage to work around a [Stencil
bug](ionic-team/stencil#4074) where the ref
node can be out of sync.

**Note**: this also updates the conventions doc with this info.
alicewriteswrongs added a commit that referenced this issue Apr 1, 2024
Sort attribute names before calling `setAccessor` with their changed
values in order to ensure that the `ref` attribute is handled by
`setAccessor` after all other attributes have been handled. This
prevents the execution order for `ref` callbacks from being dependency
on the order in which attributes are declared in the JSX.

STENCIL-737
fixes #4074
alicewriteswrongs added a commit that referenced this issue Apr 2, 2024
Sort attribute names before calling `setAccessor` with their changed
values in order to ensure that the `ref` attribute is handled by
`setAccessor` after all other attributes have been handled. This
prevents the execution order for `ref` callbacks from being dependency
on the order in which attributes are declared in the JSX.

STENCIL-737
fixes #4074
github-merge-queue bot pushed a commit that referenced this issue Apr 2, 2024
Sort attribute names before calling `setAccessor` with their changed
values in order to ensure that the `ref` attribute is handled by
`setAccessor` after all other attributes have been handled. This
prevents the execution order for `ref` callbacks from being dependency
on the order in which attributes are declared in the JSX.

STENCIL-737
fixes #4074
@rwaskiewicz
Copy link
Contributor

The fix for this issue has been released as a part of today's Stencil v4.14.1 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants