Order matters (or does it?)

[olaf-k]

tl;dr

When importing elements that depend on others in a <script src="..."> tag, make sure these dependencies are imported first.
E.g, do:

<script type="module" src="/assets/modules/components/accordion-item/index.js"></script>
<script type="module" src="/assets/modules/components/accordion/index.js"></script>

instead of:

<script type="module" src="/assets/modules/components/accordion/index.js"></script>
<script type="module" src="/assets/modules/components/accordion-item/index.js"></script>

So pay attention to the order in the doc's components.json.

But it worked so far?

Yes, but it doesn't if you make Accordion inherit from the FAST element, instead of creating it from scratch.

? 🤔

I know, right? I found out the hard way while prototyping the FAST-descending-Accordion. The test page I used locally worked well and, before opening a PR, the README needed to be updated to reflect the API changes. Except the samples in the README suddenly didn't work.
With.
The.
Same.
Code.

?? 😅

It turned out that, inside the sample, the FAST Accordion's code was failing to correctly identify and loop over its AccordionItem children. The following test was never true:

if (item instanceof FASTAccordionItem) {
    // do some stuff with the item
}

And sure enough, in that case item.__proto__ revealed itself to be HTMLElement rather than AccordionItem.
OK, but why was it working in my test page?

??? 😓

I knew instanceof could have weird results when used inside iframes, which is where our samples live. But it wasn't relevant here (the AccordionItem was defined inside the frame and item was identified as an HTMLElement anyway).

The only tiny difference between the code in my test page and the sample's one was how components were loaded:

• in the test page:

<script type="module">
    import '/assets/modules/components/accordion/index.js';
    import '/assets/modules/components/accordion-item/index.js';
</script>

• in the sample:

<script type="module" src="/assets/modules/components/accordion/index.js"></script>
<script type="module" src="/assets/modules/components/accordion-item/index.js"></script>

Both of these snippets are functionally equivalent: they import the component code and register it.
They are, however, processed differently.

Here's the timeline of events in the test page:

...and in the sample:

Oooh... 💡

So it seems that in the test page, where components are all imported "manually" inside a <script> tag, they only start to be defined in the registry once all of them are loaded. On the other hand, in the sample, where components are imported in their own <script> tag, each of them is immediately registered as soon as it is loaded.

And that is a problem for our Accordion because the failing code mentioned at the beginning relies on the AccordionItem class being defined in the execution scope (even if the custom component isn't registered yet), which is the case in the test page, but not yet in the sample. The solution for it was then simply to switch the <script> tags order and voilà !

Since I spent quite a bit of time on this bug, thank you for spending yours reading this.

[yinonov]

does this relate?

vivid-3/libs/components/src/lib/menu/definition.ts

Lines 18 to 22 in 13ebc0a

	// by convention, menu-item isn't required to be imported
	// in menu as it is not used directly in template, rather by user's authoring.
	// but, due to the race condition and way menu needs children to
	// connect before setting/checking their props/attributes, it is required
	/**

[olaf-k]

Might be?

If the sample fails when you don't add MenuItem to the definition file, you can try inverting the dependencies order in components.json and check if that solves the issue.

[yinonov]

it may fall in between the cracks when it comes to authors' concern. won't it be better to simply import items within they're containing widget? true, it's not included in the template, however, it is included in the class logic.

[olaf-k]

Whatever technical solution is chosen, it is only hiding the fact that a container component is built with an internal references to its children.

If someday you want to rename AccordionItem or plug other expandable type of children in an Accordion, you'll need to modify its code. I would prefer getting rid of instanceof altogether and rely on interface checking ("does this child has an expand prop? / does it have the aria-role it should? / etc.").

[yinonov]

agree. but that's outside the scope of Vivid. such issue should be opened in https://github.com/microsoft/fast. well, I guess that what you're suggesting

[olaf-k]

I was actually confusing myself on my own. Using duck typing instead of instanceof would not solve the fact that AccordionItem needs to be in scope, otherwise you're just manipulating the not-yet-upgraded element.