Possible bug on `ViewDesc.posBeforeChild`

Hey @marijn, is this change on posBeforeChild intentional?

// Before TypeScript port (1.23.13)
  posBeforeChild(child) {
    for (let i = 0, pos = this.posAtStart; i < this.children.length; i++) {
// After TypeScript port (1.24.0)
  posBeforeChild(child: ViewDesc): number {
    for (let i = 0, pos = this.posAtStart;; i++) {

Now, the for loop isn’t stoping when the counter is bigger than the amount of children. As result, cur is getting null and the operation cur.size is throwing an error.

How is this being called with a node that’s not a child of the target node?

We have some CustomNodeViews using React internally. So, in some moment a getPos() is being called during the re-render.

I still need to do a proper investigation how we can create a simple reproduce steps for it. For now, I am trying to understand if that change in the for-loop was intentional.

The return type is number, not number | undefined, so yeah, that function isn’t supposed to be called on things that aren’t children.

Hey @marijn, sorry it took so long to answer you.

Could you tell me if I misunderstood the getPos implementation?

() => {
      // (This is a function that allows the custom view to find its
      // own position)
      if (!descObj) return pos
      if (descObj.parent) return descObj.parent.posBeforeChild(descObj)

Is this the getPos implemented in the NodeViewDesc right? There is a branch “returning” undefined during runtime. That why I understood the getPos returning “undefined” in some weird scenarios.

Edit: I checked the official type definition and the ‘() => number | undefined’ is there too. For one second I thought I was using the previously non official type.

/// The type of function [provided](#view.EditorProps.nodeViews) to
/// create [node views](#view.NodeView).
export type NodeViewConstructor = (node: Node, view: EditorView, getPos: () => number | undefined,

Now you’re talking about getPos. I was talking about the return type of posBeforeChild.

I see. Sorry, I should be more clear.

That was how I was having the issue. A NodeView was calling the getPos and that exception was happening.

Before, we would get a ‘undefined’ instead of Error

If a view desc has a parent, it should be a child of that parent. If that’s being violated somewhere, the bug is that violation. If you have an example that reproduces this, we could investigate it.