Skip to content

Commit

Permalink
fix(runtime): prevent ref callbacks from being called too early (#5614)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alicewriteswrongs committed Apr 2, 2024
1 parent 363c07b commit 81fa375
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 2 deletions.
23 changes: 21 additions & 2 deletions src/runtime/vdom/update-element.ts
Expand Up @@ -23,15 +23,34 @@ export const updateElement = (

if (BUILD.updatable) {
// remove attributes no longer present on the vnode by setting them to undefined
for (memberName in oldVnodeAttrs) {
for (memberName of sortedAttrNames(Object.keys(oldVnodeAttrs))) {
if (!(memberName in newVnodeAttrs)) {
setAccessor(elm, memberName, oldVnodeAttrs[memberName], undefined, isSvgMode, newVnode.$flags$);
}
}
}

// add new & update changed attributes
for (memberName in newVnodeAttrs) {
for (memberName of sortedAttrNames(Object.keys(newVnodeAttrs))) {
setAccessor(elm, memberName, oldVnodeAttrs[memberName], newVnodeAttrs[memberName], isSvgMode, newVnode.$flags$);
}
};

/**
* Sort a list of attribute names to ensure that all the attribute names which
* are _not_ `"ref"` come before `"ref"`. Preserve the order of the non-ref
* attributes.
*
* **Note**: if the supplied attributes do not include `'ref'` then the same
* (by reference) array will be returned without modification.
*
* @param attrNames attribute names to sort
* @returns a list of attribute names, sorted if they include `"ref"`
*/
function sortedAttrNames(attrNames: string[]): string[] {
return attrNames.includes('ref')
? // we need to sort these to ensure that `'ref'` is the last attr
[...attrNames.filter((attr) => attr !== 'ref'), 'ref']
: // no need to sort, return the original array
attrNames;
}
16 changes: 16 additions & 0 deletions test/wdio/ref-attr-order/cmp.test.tsx
@@ -0,0 +1,16 @@
import { h } from '@stencil/core';
import { render } from '@wdio/browser-runner/stencil';

describe('ref-attr-order', () => {
beforeEach(() => {
render({
template: () => <ref-attr-order></ref-attr-order>,
});
});

it('should call the `ref` callback after handling other attrs', async () => {
const cmp = await $('ref-attr-order');
await cmp.waitForStable();
await expect(cmp).toHaveText('my tabIndex: 0');
});
});
28 changes: 28 additions & 0 deletions test/wdio/ref-attr-order/cmp.tsx
@@ -0,0 +1,28 @@
import { Component, h, State } from '@stencil/core';

@Component({
tag: 'ref-attr-order',
shadow: true,
})
export class RefAttrOrder {
@State() index: number = -1;

// order matters for the attributes in the test below!
//
// this is testing that even though the `ref` attribute is declared first in
// the JSX for the `div` the `ref` callback will nonetheless be called after
// the `tabIndex` attribute is applied to the element.
// See https://github.com/ionic-team/stencil/issues/4074
render() {
return (
<div
ref={(el) => {
this.index = el.tabIndex;
}}
tabIndex={0}
>
my tabIndex: {this.index}
</div>
);
}
}

0 comments on commit 81fa375

Please sign in to comment.