Does changing tag to be the union of string | null also have implications for VNode and ChildNode? We assign tag to the new VNode.$tag$ field immediately here, without any validation/default values.

This widening also affects ChildNode, since it's our "friendlier named" version of the data structure.

While we're in here, I would recommend we update those types. IIUC, these fields could have always been null, so I'm not too hesitant about this being considered a "breaking change". We may want to make this a part of v3.1.0 to play it a little safer though (as opposed to some patch version of v3.0)

I believe this would also have some implications in other parts of the runtime code. At the very least, it would widen types of variables that reference either of these fields. We might be handling that OK today (after all, this is just a type checker where the types go away at compile time). However, it may be worth tracing the usages of these two fields on VNode and ChildNode if we are indeed going to be widening them.

Yeah, that makes sense. I made the change basically because we are explicitly passing null to both of these parameters in a few places, e.g. here:

https://github.com/ionic-team/stencil/blob/9ace5cd87cd1ed1380087fedd8d7c53e043afc12/src/runtime/vdom/h.ts#L90-L90

and here:

https://github.com/ionic-team/stencil/blob/9ace5cd87cd1ed1380087fedd8d7c53e043afc12/src/runtime/vdom/h.ts#L44-L44

So I figured the typing for the parameters should be inline with that! I'm going to adjust the types for VNode and ChildNode to match

My guess is that this was originally here to prevent additional allocations of "whatever" childNode gets assigned to. Not sure if that's the case, nor do I think we necessarily need to change it back - just some food for thought

One thing we should verify - I'm oversimplifying the issue a little bit, but in TS it's possible to assign a property to an array and have the type checker be OK with it:

const foo: any = [];
foo.$tag$ = 'bar';

With our change to isHost, the previous implementation (of isHost) could have returned true and narrowed the type of renderFnResults (since the original implementation was a type guard), even if renderFnResults was an array (assuming it had a $tag$ property on it, which i believe only could happen in the situation I described above.

Can you do me a favor and verify the call chain to renderVdom to verify that something like this shouldn't happen? I'm not sure if there's any type erasure or widening in that chain that could lead to an issue here, but probably best be safe than sorry.

It's hard to say 100% because unfortunately we more or less have typechecking turned off in update-component.ts (lots of any scattered around 😞) but looking through the call sites for renderVdom, callRender, and updateComponent I don't see anything like that going on. The reason I opened this PR in the first place was in part that I get a little scared of how un-checked some of these files are...

Fair, so long as we're fairly confident here. Worst case scenario, we need to revert this (and if there is a rendering bug, we know where to look first)

Does widening this to null have any affect on Functional Components? Namely, does this affect the FunctionUtilities#forEach function? I think that was originally written under the assumption that this property would always be an array (even if it were empty), but I'm not 100% sure of the fallout of making this null.