"Index out of bounds" error

[jthemphill]

I have code like this:

function MultiAllotment({...}) {
  return <Allotment
        ref={allotmentRef}
        vertical={split === 'vertical'}
        defaultSizes={defaultSizes}
        separator={!hideSeparator}
        onVisibleChange={(index, visible) => panes[index].onVisibleChange?.(visible)}
        onChange={(sizes) => panes.forEach((pane, index) => pane.onChange?.(sizes[index]))}
        onDragEnd={(sizes) => panes.forEach((pane, index) => pane.onDragEnd?.(sizes[index]))}
        className={styles.main}
      >
        {panes.map(({ id, visible, snap, preferredSize, minSize, maxSize, className, children }) => (
          <Allotment.Pane
            key={id}
            visible={visible}
            snap={snap}
            preferredSize={preferredSize}
            minSize={minSize}
            maxSize={maxSize}
            className={className}
          >
            {children}
          </Allotment.Pane>
        ))}
      </Allotment>
}

It's failing here:

allotment/src/split-view/split-view.ts

Lines 548 to 550 in 451dfb6

	if (index < 0 || index >= this.viewItems.length) {
	throw new Error("Index out of bounds");
	}

This is because we're calling isViewVisible() for each index in childrenArray:

allotment/src/allotment.tsx

Line 405 in 451dfb6

splitViewRef.current?.isViewVisible(index) !== props.visible

...which means we'll throw an error if our SplitView has fewer viewItems than the childrenArray we passed to the Allotment. Whenever the exception is thrown, the SplitView has an empty viewItems array.

But... I don't know why we should expect SplitView to have the same number of viewItems as childrenArray, or why we shouldn't expect viewItems to be nonempty. Is there any guarantee that these arrays are the same size or that the viewItems will be nonempty? I can't find anything in the documentation that would imply that I'm using Allotment incorrectly.

If there is no guarantee, should we really be throwing an exception? This is a predicate function. Can it return false instead?

[mletic]

I recently had the same "Index out of bounds" error when I was dynamically setting visibility on the Allotment.Pane.
My main issue was that I forgot to add the defaultSizes array on the parent Allotment component.

Maybe check if your defaultSizes.length is the same as panes.length in your code, and if not make sure you fix this so that whenever the panes.length change you also update the defaultSizes array.

Not sure if this will solve your problem but it's worth a shot.

[jthemphill]

If this is an invariant that's required in order to use Allotment correctly, then it would be good to check this invariant and report it to the user! Error messages should be actionable and should tell us how to use the library correctly!

I added

if (defaultSizes && defaultSizes.length !== panes.length) {
  throw new Error(`defaultSizes has length ${defaultSizes.length}, but panes has length ${panes.length}`)
}

to my code which uses Allotment, but this error is never thrown.

[johnwalley]

Thanks for the in-depth description of what's going on 🙏.

I'll look into it and see what I can improve when I next find some time (probably in a couple of weeks).

Chances are it's an oversight from my attempts to interface the React part of the codebase to the vscode part. You can see the vscode original exception here: https://github.com/microsoft/vscode/blob/bd4ab867f9ca31105f05032cf09edbce31fc6fe3/src/vs/base/browser/ui/splitview/splitview.ts#L793

[jthemphill]

Here's a rundown of what I did to workaround the problem in our production repo. Sorry that this isn't a minimal repro... I would be just as happy if the next release of Allotment just contained some hardening so it's easier to debug this issue and create a minimal repro if we run into this problem again.

In my particular repo, I was able to isolate the problem to a lazy component which begins loading after Allotment renders. I was able to suppress the issue in our production repo either by making the lazy component load eagerly, or by wrapping the lazy component inside of a <Suspense> tag.

The tree looks something like this:

<Suspense fallback={<Spinner />}>
  <Allotment>
    <Allotment.Pane>
      <Editor>
        {shouldRenderNux && React.lazy(() => import('./NewUserExperienceModal'))}
        <OtherEditorStuff />
      </Editor>
    </Allotment.Pane>
    <Allotment.Pane>
      <OtherPaneContent />
    </Allotment.Pane>
  </Allotment>
</Suspense>

Note that shouldRenderNux is initially false, and then later becomes true. Forcing shouldRenderNux to always be false or always be true will suppress the issue as well.

With my fix, the tree looks like:

<Suspense fallback={<Spinner />}>
  <Allotment>
    <Allotment.Pane>
      <Editor>
        <Suspense fallback={null}>
          {shouldRenderNux && React.lazy(() => import('./NewUserExperienceModal'))}
        </Suspense>
        <OtherEditorStuff />
      </Editor>
    </Allotment.Pane>
    <Allotment.Pane>
      <OtherPaneContent />
    </Allotment.Pane>
  </Allotment>
</Suspense>

Normally, <Suspense fallback={null}> doesn't actually do very much - the purpose of a <Suspense> tag is to render a fallback instead of your lazy component. But it's not quite a no-op! When a component is lazily rendering, React will look for the nearest parent <Suspense /> and replace all of its children with the fallback. By wrapping the lazy component in a <Suspense> tag, I limited the scope of the damage to just the loading modal, thus not unmounting <Allotment> or any of its Panes.