virtual-scroller - npm Package Compare versions

Comparing version 1.12.2 to 1.12.3

CHANGELOG.md

 <!-- `virtual-scroller`: in `.updateItems()` handle a case when `items.length` is the same, in which case find different items and if those items are rendered then maybe update them on screen and update their height, if the items are past rendered then maybe just discard all item heights past rendered, if the items are before rendered then maybe ignore and it will jump on scroll up which is kinda acceptable. -->
 
+1.12.3 / 23.03.2023
+==================
+
+* [Fixed](https://gitlab.com/catamphetamine/virtual-scroller/-/issues/33) React `18.2.0` [bug](https://github.com/facebook/react/issues/26320) in ["strict" mode](https://hu.reactjs.org/docs/strict-mode.html) when `useInsertionEffect()` doesn't run twice on mount unlike `useEffect()` or `useLayoutEffect()`.
+
+* Fixed React `18.2.0` [bug](https://github.com/facebook/react/issues/25023#issuecomment-1480463544) when out-of-sync (stale) state values are being rendered.
+
+* `VirtualScroller` no longer restores the Y scroll position on mount: it was found out that this feature conflicted with the same feature of application "router" libraries.
+
 1.11.3 / 05.02.2023
@@ -4,0 +13,0 @@ ==================

commonjs/Layout.js

@@ -89,3 +89,3 @@ "use strict";
         if (error instanceof _ScrollableContainerNotReadyError["default"]) {
-          (0, _debug["default"])('Couldn\'t calculate', name, 'before scrollable container is ready. Default to', defaultValue);
+          (0, _debug["default"])("Scrollable container size is not known at this point, so \"".concat(name, "\" can't be calculated yet. Default to"), defaultValue);
           return defaultValue;
@@ -92,0 +92,0 @@ } else {

commonjs/react/useState.js

@@ -16,2 +16,8 @@ "use strict";
 
+var _useStateNoStaleBug3 = _interopRequireDefault(require("./useStateNoStaleBug.js"));
+
+var _useInsertionEffectDontMountTwiceInStrictMode = _interopRequireDefault(require("./useInsertionEffectDontMountTwiceInStrictMode.js"));
+
+var _useLayoutEffectDontMountTwiceInStrictMode = _interopRequireDefault(require("./useLayoutEffectDontMountTwiceInStrictMode.js"));
+
 function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { "default": obj }; }
@@ -46,6 +52,6 @@
   // correspond exactly to what's currently rendered on the screen.
-  var _useState2 = (0, _react.useState)(initialState),
-      _useState3 = _slicedToArray(_useState2, 2),
-      _newState = _useState3[0],
-      _setNewState = _useState3[1]; // This `state` reference is what `VirtualScroller` uses internally.
+  var _useStateNoStaleBug = (0, _useStateNoStaleBug3["default"])(initialState),
+      _useStateNoStaleBug2 = _slicedToArray(_useStateNoStaleBug, 2),
+      _newState = _useStateNoStaleBug2[0],
+      _setNewState = _useStateNoStaleBug2[1]; // This `state` reference is what `VirtualScroller` uses internally.
   // It's the "source of truth" on the actual `VirtualScroller` state.
@@ -168,3 +174,3 @@
 
-  (0, _react.useInsertionEffect)(function () {
+  (0, _useInsertionEffectDontMountTwiceInStrictMode["default"])(function () {
     // Update the actual `VirtualScroller` state right before the DOM changes
@@ -192,3 +198,3 @@ // are going to be applied for the requested state update.
     if ((0, _debug.isDebug)()) {
-      (0, _debug["default"])('React: ~ The requested state is about to be applied in DOM. Set it as the `VirtualScroller` state. ~');
+      (0, _debug["default"])('React: ~ The requested state is about to be applied in DOM. Setting it as the `VirtualScroller` state. ~');
       (0, _debug["default"])((0, _getStateSnapshot["default"])(_newState));
@@ -199,3 +205,3 @@ }
   }, [_newState]);
-  (0, _react.useLayoutEffect)(function () {
+  (0, _useLayoutEffectDontMountTwiceInStrictMode["default"])(function () {
     // Call `onRender()` right after a requested state update has been applied,
@@ -202,0 +208,0 @@ // and also right after the initial render.

commonjs/react/VirtualScroller.js

@@ -36,2 +36,4 @@ "use strict";
 
+var _debug = require("../utility/debug.js");
+
 var _excluded = ["as", "items", "itemComponent", "itemComponentProps", "estimatedItemHeight", "getEstimatedItemHeight", "getEstimatedVisibleItemRowsCount", "bypass", "tbody", "preserveScrollPosition", "preserveScrollPositionOnPrependItems", "measureItemsBatchSize", "scrollableContainer", "getScrollableContainer", "getColumnsCount", "getItemId", "className", "onMount", "onItemFirstRender", "onItemInitialRender", "initialScrollPosition", "onScrollPositionChange", "onStateChange", "initialState", "getInitialItemState"];
@@ -191,2 +193,3 @@
     if (onMount) {
+      (0, _debug.warn)('`onMount` property is deprecated');
       onMount();
@@ -193,0 +196,0 @@ }

commonjs/Scroll.js

@@ -142,3 +142,4 @@ "use strict";
       if (this.initialScrollPosition !== undefined) {
-        this.scrollToY(this.initialScrollPosition);
+        this.scrollToY(this.initialScrollPosition); // Don't restore this scroll position on restart.
+
         this.initialScrollPosition = undefined;
@@ -145,0 +146,0 @@ }

commonjs/VirtualScroller.js

@@ -117,4 +117,9 @@ "use strict";
 
-      (0, _debug["default"])('~ Start ~'); // `this._isActive = true` should be placed somewhere at the start of this function.
+      if (isRestart) {
+        (0, _debug["default"])('~ Start (restart) ~');
+      } else {
+        (0, _debug["default"])('~ Start ~');
+      } // `this._isActive = true` should be placed somewhere at the start of this function.
 
+
       this._isActive = true; // Reset `ListHeightMeasurement` just in case it has some "leftover" state.
@@ -133,16 +138,11 @@
         }
-      } // If there was a pending state update that didn't get applied
-      // because of stopping the `VirtualScroller`, apply that state update now.
-      //
-      // The pending state update won't get applied if the scrollable container width
-      // has changed but that's ok because that state update currently could only contain:
-      // * `scrollableContainerWidth`
-      // * `verticalSpacing`
-      // * `beforeResize`
-      // All of those get rewritten in `onResize()` anyway.
-      //
+      } // If there was a pending "after render" state update that didn't get applied
+      // because the `VirtualScroller` got stopped, then apply that pending "after render"
+      // state update now. Such state update could include properties like:
+      // * A `verticalSpacing` that has been measured in `onRender()`.
+      // * A cleaned-up `beforeResize` object that was cleaned-up in `onRender()`.
 
 
-      var stateUpdate = this._stoppedStateUpdate;
-      this._stoppedStateUpdate = undefined; // Reset `this.verticalSpacing` so that it re-measures it in cases when
+      var stateUpdate = this._afterRenderStateUpdateThatWasStopped;
+      this._afterRenderStateUpdateThatWasStopped = undefined; // Reset `this.verticalSpacing` so that it re-measures it in cases when
       // the `VirtualScroller` was previously stopped and is now being restarted.
@@ -181,8 +181,6 @@ // The rationale is that a previously captured inter-item vertical spacing
         if (newWidth !== prevWidth) {
-          (0, _debug["default"])('~ Scrollable container width changed from', prevWidth, 'to', newWidth, '~'); // `stateUpdate` doesn't get passed to `this.onResize()`, and, therefore,
-          // won't be applied. But that's ok because currently it could only contain:
-          // * `scrollableContainerWidth`
-          // * `verticalSpacing`
-          // * `beforeResize`
-          // All of those get rewritten in `onResize()` anyway.
+          (0, _debug["default"])('~ Scrollable container width changed from', prevWidth, 'to', newWidth, '~'); // The pending state update (if present) won't be applied in this case.
+          // That's ok because such state update could currently only originate in
+          // `this.onResize()` function. Therefore, alling `this.onResize()` again
+          // would rewrite all those `stateUpdate` properties anyway, so they're not passed.
 
@@ -261,4 +259,5 @@ return this.onResize();
     value: function onItemHeightDidChange(i) {
-      this.hasToBeStarted();
-
+      // See the comments in the `setItemState()` function below for the rationale
+      // on why the `hasToBeStarted()` check was commented out.
+      // this.hasToBeStarted()
       this._onItemHeightDidChange(i);
@@ -275,4 +274,44 @@ }
     value: function setItemState(i, newItemState) {
-      this.hasToBeStarted();
-
+      // There is an issue in React 18.2.0 when `useInsertionEffect()` doesn't run twice
+      // on mount unlike `useLayoutEffect()` in "strict" mode. That causes a bug in a React
+      // implementation of the `virtual-scroller`.
+      // https://gitlab.com/catamphetamine/virtual-scroller/-/issues/33
+      // https://github.com/facebook/react/issues/26320
+      // A workaround for that bug is ignoring the second-initial run of the effects at mount.
+      //
+      // But in that case, if an `ItemComponent` calls `setItemState()` in `useLayoutEffect()`,
+      // it could result in a bug.
+      //
+      // Consider a type of `useLayoutEffect()` that skips the initial mount:
+      // `useLayoutEffectSkipInitialMount()`.
+      // Suppose that effect is written in such a way that it only skips the first call of itself.
+      // In that case, if React is run in "strict" mode, the effect will no longer work as expected
+      // and it won't actually skip the initial mount and will be executed during the second initial run.
+      // But the `VirtualScroller` itself has already implemented a workaround that prevents
+      // its hooks from running twice on mount. This means that `useVirtualScrollerStartStop()`
+      // of the React component would have already stopped the `VirtualScroller` by the time
+      // `ItemComponent`'s incorrectly-behaving `useLayoutEffectSkipInitialMount()` effect is run,
+      // resulting in an error: "`VirtualScroller` hasn't been started".
+      //
+      // The log when not in "strict" mode would be:
+      //
+      // * `useLayoutEffect()` is run in `ItemComponent` — skips the initial run.
+      // * `useLayoutEffect()` is run in `useVirtualScrollerStartStop()`. It starts the `VirtualScroller`.
+      // * Some dependency property gets updated inside `ItemComponent`.
+      // * `useLayoutEffect()` is run in `ItemComponent` — no longer skips. Calls `setItemState()`.
+      // * The `VirtualScroller` is started so it handles `setState()` correctly.
+      //
+      // The log when in "strict" mode would be:
+      //
+      // * `useLayoutEffect()` is run in `ItemComponent` — skips the initial run.
+      // * `useLayoutEffect()` is run in `useVirtualScrollerStartStop()`. It starts the `VirtualScroller`.
+      // * `useLayoutEffect()` is unmounted in `useVirtualScrollerStartStop()`. It stops the `VirtualScroller`.
+      // * `useLayoutEffect()` is unmounted in `ItemComponent` — does nothing.
+      // * `useLayoutEffect()` is run the second time in `ItemComponent` — no longer skips. Calls `setItemState()`.
+      // * The `VirtualScroller` is stopped so it throws an error: "`VirtualScroller` hasn't been started".
+      //
+      // For that reason, the requirement of the `VirtualScroller` to be started was commented out.
+      // Commenting it out wouldn't result in any potential bugs because the code would work correctly
+      // in both cases.
+      // this.hasToBeStarted()
       this._setItemState(i, newItemState);
@@ -279,0 +318,0 @@ } // (deprecated)

commonjs/VirtualScroller.layout.js

@@ -468,5 +468,9 @@ "use strict";
     if (previousHeight !== newHeight) {
-      (0, _debug["default"])('~ Item height has changed. Should update layout. ~'); // Update or reset a previously calculated layout
-      // so that the "diff"s based on that layout in the future
-      // produce correct results.
+      (0, _debug["default"])('~ Item height has changed. Should update layout. ~'); // Update or reset a previously calculated layout with the new item height
+      // so that the potential future "diff"s based on that "previously calculated" layout
+      // would be correct.
+      //
+      // The "previously calculated layout" feature is not currently used
+      // so this function call doesn't really affect anything.
+      //
 
@@ -483,9 +487,11 @@ updatePreviouslyCalculatedLayoutOnItemHeightChange.call(_this, i, previousHeight, newHeight); // Recalculate layout.
 
-      if (_this.waitingForRender) {
-        (0, _debug["default"])('~ Another state update is already waiting to be rendered. Delay the layout update until then. ~');
-        _this.updateLayoutAfterRenderBecauseItemHeightChanged = true;
-      } else {
-        _this.onUpdateShownItemIndexes({
-          reason: _Layout.LAYOUT_REASON.ITEM_HEIGHT_CHANGED
-        });
+      if (_this._isActive) {
+        if (_this.waitingForRender) {
+          (0, _debug["default"])('~ Another state update is already waiting to be rendered. Delay the layout update until then. ~');
+          _this.updateLayoutAfterRenderBecauseItemHeightChanged = true;
+        } else {
+          _this.onUpdateShownItemIndexes({
+            reason: _Layout.LAYOUT_REASON.ITEM_HEIGHT_CHANGED
+          });
+        }
       } // If there was a request for `setState()` with new `items`, then the changes
@@ -492,0 +498,0 @@ // to `currentState.itemHeights[]` made above in a `remeasureItemHeight()` call

commonjs/VirtualScroller.onRender.js

@@ -89,3 +89,3 @@ "use strict";
         (0, _debug.warn)('The most recent state that was set', (0, _getStateSnapshot["default"])(_this.mostRecentSetStateValue));
-        (0, _debug.reportError)('The state that has been rendered is not the most recent one that was set');
+        (0, _debug.reportError)('`VirtualScroller` has been rendered with a `state` that is not equal to the most recently set one');
       }
@@ -219,3 +219,3 @@ } // `this.resetStateUpdateFlags()` must be called before calling
     if (!_this._isActive) {
-      _this._stoppedStateUpdate = stateUpdate;
+      _this._afterRenderStateUpdateThatWasStopped = stateUpdate;
       return;
@@ -222,0 +222,0 @@ }

modules/Layout.js

@@ -73,3 +73,3 @@ function ownKeys(object, enumerableOnly) { var keys = Object.keys(object); if (Object.getOwnPropertySymbols) { var symbols = Object.getOwnPropertySymbols(object); enumerableOnly && (symbols = symbols.filter(function (sym) { return Object.getOwnPropertyDescriptor(object, sym).enumerable; })), keys.push.apply(keys, symbols); } return keys; }
         if (error instanceof ScrollableContainerNotReadyError) {
-          log('Couldn\'t calculate', name, 'before scrollable container is ready. Default to', defaultValue);
+          log("Scrollable container size is not known at this point, so \"".concat(name, "\" can't be calculated yet. Default to"), defaultValue);
           return defaultValue;
@@ -76,0 +76,0 @@ } else {

modules/react/useState.js

@@ -15,3 +15,6 @@ function _slicedToArray(arr, i) { return _arrayWithHoles(arr) || _iterableToArrayLimit(arr, i) || _unsupportedIterableToArray(arr, i) || _nonIterableRest(); }
 import getStateSnapshot from '../utility/getStateSnapshot.js';
-import { useState, useRef, useCallback, useLayoutEffect, useInsertionEffect } from 'react'; // Creates state management functions.
+import { useRef, useCallback } from 'react';
+import useStateNoStaleBug from './useStateNoStaleBug.js';
+import useInsertionEffectDontMountTwiceInStrictMode from './useInsertionEffectDontMountTwiceInStrictMode.js';
+import useLayoutEffectDontMountTwiceInStrictMode from './useLayoutEffectDontMountTwiceInStrictMode.js'; // Creates state management functions.
 
@@ -28,6 +31,6 @@ export default function _useState(_ref) {
   // correspond exactly to what's currently rendered on the screen.
-  var _useState2 = useState(initialState),
-      _useState3 = _slicedToArray(_useState2, 2),
-      _newState = _useState3[0],
-      _setNewState = _useState3[1]; // This `state` reference is what `VirtualScroller` uses internally.
+  var _useStateNoStaleBug = useStateNoStaleBug(initialState),
+      _useStateNoStaleBug2 = _slicedToArray(_useStateNoStaleBug, 2),
+      _newState = _useStateNoStaleBug2[0],
+      _setNewState = _useStateNoStaleBug2[1]; // This `state` reference is what `VirtualScroller` uses internally.
   // It's the "source of truth" on the actual `VirtualScroller` state.
@@ -150,3 +153,3 @@
 
-  useInsertionEffect(function () {
+  useInsertionEffectDontMountTwiceInStrictMode(function () {
     // Update the actual `VirtualScroller` state right before the DOM changes
@@ -174,3 +177,3 @@ // are going to be applied for the requested state update.
     if (isDebug()) {
-      log('React: ~ The requested state is about to be applied in DOM. Set it as the `VirtualScroller` state. ~');
+      log('React: ~ The requested state is about to be applied in DOM. Setting it as the `VirtualScroller` state. ~');
       log(getStateSnapshot(_newState));
@@ -181,3 +184,3 @@ }
   }, [_newState]);
-  useLayoutEffect(function () {
+  useLayoutEffectDontMountTwiceInStrictMode(function () {
     // Call `onRender()` right after a requested state update has been applied,
@@ -184,0 +187,0 @@ // and also right after the initial render.

modules/react/VirtualScroller.js

@@ -21,3 +21,4 @@ var _excluded = ["as", "items", "itemComponent", "itemComponentProps", "estimatedItemHeight", "getEstimatedItemHeight", "getEstimatedVisibleItemRowsCount", "bypass", "tbody", "preserveScrollPosition", "preserveScrollPositionOnPrependItems", "measureItemsBatchSize", "scrollableContainer", "getScrollableContainer", "getColumnsCount", "getItemId", "className", "onMount", "onItemFirstRender", "onItemInitialRender", "initialScrollPosition", "onScrollPositionChange", "onStateChange", "initialState", "getInitialItemState"];
 import useClassName from './useClassName.js';
-import useStyle from './useStyle.js'; // When `items` property changes:
+import useStyle from './useStyle.js';
+import { warn } from '../utility/debug.js'; // When `items` property changes:
 // * A new `items` property is supplied to the React component.
@@ -163,2 +164,3 @@ // * The React component re-renders itself.
     if (onMount) {
+      warn('`onMount` property is deprecated');
       onMount();
@@ -165,0 +167,0 @@ }

modules/Scroll.js

@@ -136,3 +136,4 @@ function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }
       if (this.initialScrollPosition !== undefined) {
-        this.scrollToY(this.initialScrollPosition);
+        this.scrollToY(this.initialScrollPosition); // Don't restore this scroll position on restart.
+
         this.initialScrollPosition = undefined;
@@ -139,0 +140,0 @@ }

modules/VirtualScroller.js

@@ -99,4 +99,9 @@ function ownKeys(object, enumerableOnly) { var keys = Object.keys(object); if (Object.getOwnPropertySymbols) { var symbols = Object.getOwnPropertySymbols(object); enumerableOnly && (symbols = symbols.filter(function (sym) { return Object.getOwnPropertyDescriptor(object, sym).enumerable; })), keys.push.apply(keys, symbols); } return keys; }
 
-      log('~ Start ~'); // `this._isActive = true` should be placed somewhere at the start of this function.
+      if (isRestart) {
+        log('~ Start (restart) ~');
+      } else {
+        log('~ Start ~');
+      } // `this._isActive = true` should be placed somewhere at the start of this function.
 
+
       this._isActive = true; // Reset `ListHeightMeasurement` just in case it has some "leftover" state.
@@ -115,16 +120,11 @@
         }
-      } // If there was a pending state update that didn't get applied
-      // because of stopping the `VirtualScroller`, apply that state update now.
-      //
-      // The pending state update won't get applied if the scrollable container width
-      // has changed but that's ok because that state update currently could only contain:
-      // * `scrollableContainerWidth`
-      // * `verticalSpacing`
-      // * `beforeResize`
-      // All of those get rewritten in `onResize()` anyway.
-      //
+      } // If there was a pending "after render" state update that didn't get applied
+      // because the `VirtualScroller` got stopped, then apply that pending "after render"
+      // state update now. Such state update could include properties like:
+      // * A `verticalSpacing` that has been measured in `onRender()`.
+      // * A cleaned-up `beforeResize` object that was cleaned-up in `onRender()`.
 
 
-      var stateUpdate = this._stoppedStateUpdate;
-      this._stoppedStateUpdate = undefined; // Reset `this.verticalSpacing` so that it re-measures it in cases when
+      var stateUpdate = this._afterRenderStateUpdateThatWasStopped;
+      this._afterRenderStateUpdateThatWasStopped = undefined; // Reset `this.verticalSpacing` so that it re-measures it in cases when
       // the `VirtualScroller` was previously stopped and is now being restarted.
@@ -163,8 +163,6 @@ // The rationale is that a previously captured inter-item vertical spacing
         if (newWidth !== prevWidth) {
-          log('~ Scrollable container width changed from', prevWidth, 'to', newWidth, '~'); // `stateUpdate` doesn't get passed to `this.onResize()`, and, therefore,
-          // won't be applied. But that's ok because currently it could only contain:
-          // * `scrollableContainerWidth`
-          // * `verticalSpacing`
-          // * `beforeResize`
-          // All of those get rewritten in `onResize()` anyway.
+          log('~ Scrollable container width changed from', prevWidth, 'to', newWidth, '~'); // The pending state update (if present) won't be applied in this case.
+          // That's ok because such state update could currently only originate in
+          // `this.onResize()` function. Therefore, alling `this.onResize()` again
+          // would rewrite all those `stateUpdate` properties anyway, so they're not passed.
 
@@ -243,4 +241,5 @@ return this.onResize();
     value: function onItemHeightDidChange(i) {
-      this.hasToBeStarted();
-
+      // See the comments in the `setItemState()` function below for the rationale
+      // on why the `hasToBeStarted()` check was commented out.
+      // this.hasToBeStarted()
       this._onItemHeightDidChange(i);
@@ -257,4 +256,44 @@ }
     value: function setItemState(i, newItemState) {
-      this.hasToBeStarted();
-
+      // There is an issue in React 18.2.0 when `useInsertionEffect()` doesn't run twice
+      // on mount unlike `useLayoutEffect()` in "strict" mode. That causes a bug in a React
+      // implementation of the `virtual-scroller`.
+      // https://gitlab.com/catamphetamine/virtual-scroller/-/issues/33
+      // https://github.com/facebook/react/issues/26320
+      // A workaround for that bug is ignoring the second-initial run of the effects at mount.
+      //
+      // But in that case, if an `ItemComponent` calls `setItemState()` in `useLayoutEffect()`,
+      // it could result in a bug.
+      //
+      // Consider a type of `useLayoutEffect()` that skips the initial mount:
+      // `useLayoutEffectSkipInitialMount()`.
+      // Suppose that effect is written in such a way that it only skips the first call of itself.
+      // In that case, if React is run in "strict" mode, the effect will no longer work as expected
+      // and it won't actually skip the initial mount and will be executed during the second initial run.
+      // But the `VirtualScroller` itself has already implemented a workaround that prevents
+      // its hooks from running twice on mount. This means that `useVirtualScrollerStartStop()`
+      // of the React component would have already stopped the `VirtualScroller` by the time
+      // `ItemComponent`'s incorrectly-behaving `useLayoutEffectSkipInitialMount()` effect is run,
+      // resulting in an error: "`VirtualScroller` hasn't been started".
+      //
+      // The log when not in "strict" mode would be:
+      //
+      // * `useLayoutEffect()` is run in `ItemComponent` — skips the initial run.
+      // * `useLayoutEffect()` is run in `useVirtualScrollerStartStop()`. It starts the `VirtualScroller`.
+      // * Some dependency property gets updated inside `ItemComponent`.
+      // * `useLayoutEffect()` is run in `ItemComponent` — no longer skips. Calls `setItemState()`.
+      // * The `VirtualScroller` is started so it handles `setState()` correctly.
+      //
+      // The log when in "strict" mode would be:
+      //
+      // * `useLayoutEffect()` is run in `ItemComponent` — skips the initial run.
+      // * `useLayoutEffect()` is run in `useVirtualScrollerStartStop()`. It starts the `VirtualScroller`.
+      // * `useLayoutEffect()` is unmounted in `useVirtualScrollerStartStop()`. It stops the `VirtualScroller`.
+      // * `useLayoutEffect()` is unmounted in `ItemComponent` — does nothing.
+      // * `useLayoutEffect()` is run the second time in `ItemComponent` — no longer skips. Calls `setItemState()`.
+      // * The `VirtualScroller` is stopped so it throws an error: "`VirtualScroller` hasn't been started".
+      //
+      // For that reason, the requirement of the `VirtualScroller` to be started was commented out.
+      // Commenting it out wouldn't result in any potential bugs because the code would work correctly
+      // in both cases.
+      // this.hasToBeStarted()
       this._setItemState(i, newItemState);
@@ -261,0 +300,0 @@ } // (deprecated)

modules/VirtualScroller.layout.js

@@ -453,5 +453,9 @@ function ownKeys(object, enumerableOnly) { var keys = Object.keys(object); if (Object.getOwnPropertySymbols) { var symbols = Object.getOwnPropertySymbols(object); enumerableOnly && (symbols = symbols.filter(function (sym) { return Object.getOwnPropertyDescriptor(object, sym).enumerable; })), keys.push.apply(keys, symbols); } return keys; }
     if (previousHeight !== newHeight) {
-      log('~ Item height has changed. Should update layout. ~'); // Update or reset a previously calculated layout
-      // so that the "diff"s based on that layout in the future
-      // produce correct results.
+      log('~ Item height has changed. Should update layout. ~'); // Update or reset a previously calculated layout with the new item height
+      // so that the potential future "diff"s based on that "previously calculated" layout
+      // would be correct.
+      //
+      // The "previously calculated layout" feature is not currently used
+      // so this function call doesn't really affect anything.
+      //
 
@@ -468,9 +472,11 @@ updatePreviouslyCalculatedLayoutOnItemHeightChange.call(_this, i, previousHeight, newHeight); // Recalculate layout.
 
-      if (_this.waitingForRender) {
-        log('~ Another state update is already waiting to be rendered. Delay the layout update until then. ~');
-        _this.updateLayoutAfterRenderBecauseItemHeightChanged = true;
-      } else {
-        _this.onUpdateShownItemIndexes({
-          reason: LAYOUT_REASON.ITEM_HEIGHT_CHANGED
-        });
+      if (_this._isActive) {
+        if (_this.waitingForRender) {
+          log('~ Another state update is already waiting to be rendered. Delay the layout update until then. ~');
+          _this.updateLayoutAfterRenderBecauseItemHeightChanged = true;
+        } else {
+          _this.onUpdateShownItemIndexes({
+            reason: LAYOUT_REASON.ITEM_HEIGHT_CHANGED
+          });
+        }
       } // If there was a request for `setState()` with new `items`, then the changes
@@ -477,0 +483,0 @@ // to `currentState.itemHeights[]` made above in a `remeasureItemHeight()` call

modules/VirtualScroller.onRender.js

@@ -69,3 +69,3 @@ function ownKeys(object, enumerableOnly) { var keys = Object.keys(object); if (Object.getOwnPropertySymbols) { var symbols = Object.getOwnPropertySymbols(object); enumerableOnly && (symbols = symbols.filter(function (sym) { return Object.getOwnPropertyDescriptor(object, sym).enumerable; })), keys.push.apply(keys, symbols); } return keys; }
         warn('The most recent state that was set', getStateSnapshot(_this.mostRecentSetStateValue));
-        reportError('The state that has been rendered is not the most recent one that was set');
+        reportError('`VirtualScroller` has been rendered with a `state` that is not equal to the most recently set one');
       }
@@ -199,3 +199,3 @@ } // `this.resetStateUpdateFlags()` must be called before calling
     if (!_this._isActive) {
-      _this._stoppedStateUpdate = stateUpdate;
+      _this._afterRenderStateUpdateThatWasStopped = stateUpdate;
       return;
@@ -202,0 +202,0 @@ }

package.json

 {
   "name": "virtual-scroller",
-  "version": "1.12.2",
+  "version": "1.12.3",
   "description": "A component for efficiently rendering large lists of variable height items",
@@ -5,0 +5,0 @@ "main": "index.cjs",

source/Layout.js

@@ -57,3 +57,3 @@ import log, { warn } from './utility/debug.js'
 			if (error instanceof ScrollableContainerNotReadyError) {
-				log('Couldn\'t calculate', name, 'before scrollable container is ready. Default to', defaultValue);
+				log(`Scrollable container size is not known at this point, so "${name}" can't be calculated yet. Default to`, defaultValue);
 				return defaultValue
@@ -60,0 +60,0 @@ } else {

source/react/useState.js

 import log, { isDebug } from '../utility/debug.js'
 import getStateSnapshot from '../utility/getStateSnapshot.js'
 
-import { useState, useRef, useCallback, useLayoutEffect, useInsertionEffect } from 'react'
+import { useRef, useCallback } from 'react'
+import useStateNoStaleBug from './useStateNoStaleBug.js'
+import useInsertionEffectDontMountTwiceInStrictMode from './useInsertionEffectDontMountTwiceInStrictMode.js'
+import useLayoutEffectDontMountTwiceInStrictMode from './useLayoutEffectDontMountTwiceInStrictMode.js'
 
@@ -17,3 +20,3 @@ // Creates state management functions.
 	// correspond exactly to what's currently rendered on the screen.
-	const [_newState, _setNewState] = useState(initialState)
+	const [_newState, _setNewState] = useStateNoStaleBug(initialState)
 
@@ -141,3 +144,3 @@ // This `state` reference is what `VirtualScroller` uses internally.
 	//
-	useInsertionEffect(() => {
+	useInsertionEffectDontMountTwiceInStrictMode(() => {
 		// Update the actual `VirtualScroller` state right before the DOM changes
@@ -165,3 +168,3 @@ // are going to be applied for the requested state update.
 		if (isDebug()) {
-			log('React: ~ The requested state is about to be applied in DOM. Set it as the `VirtualScroller` state. ~')
+			log('React: ~ The requested state is about to be applied in DOM. Setting it as the `VirtualScroller` state. ~')
 			log(getStateSnapshot(_newState))
@@ -172,3 +175,3 @@ }
 
-	useLayoutEffect(() => {
+	useLayoutEffectDontMountTwiceInStrictMode(() => {
 		// Call `onRender()` right after a requested state update has been applied,
@@ -175,0 +178,0 @@ // and also right after the initial render.

source/react/VirtualScroller.js

@@ -16,2 +16,4 @@ import React, { useRef, useMemo, useLayoutEffect } from 'react'
 
+import { warn } from '../utility/debug.js'
+
 // When `items` property changes:
@@ -176,2 +178,3 @@ // * A new `items` property is supplied to the React component.
 		if (onMount) {
+			warn('`onMount` property is deprecated')
 			onMount()
@@ -178,0 +181,0 @@ }

source/Scroll.js

@@ -45,2 +45,3 @@ // For some weird reason, in Chrome, `setTimeout()` would lag up to a second (or more) behind.
 			this.scrollToY(this.initialScrollPosition)
+			// Don't restore this scroll position on restart.
 			this.initialScrollPosition = undefined
@@ -47,0 +48,0 @@ }

source/VirtualScroller.js

@@ -52,3 +52,7 @@ import VirtualScrollerConstructor from './VirtualScroller.constructor.js'
 
-		log('~ Start ~')
+		if (isRestart) {
+			log('~ Start (restart) ~')
+		} else {
+			log('~ Start ~')
+		}
 
@@ -75,14 +79,9 @@ // `this._isActive = true` should be placed somewhere at the start of this function.
 
-		// If there was a pending state update that didn't get applied
-		// because of stopping the `VirtualScroller`, apply that state update now.
-		//
-		// The pending state update won't get applied if the scrollable container width
-		// has changed but that's ok because that state update currently could only contain:
-		// * `scrollableContainerWidth`
-		// * `verticalSpacing`
-		// * `beforeResize`
-		// All of those get rewritten in `onResize()` anyway.
-		//
-		let stateUpdate = this._stoppedStateUpdate
-		this._stoppedStateUpdate = undefined
+		// If there was a pending "after render" state update that didn't get applied
+		// because the `VirtualScroller` got stopped, then apply that pending "after render"
+		// state update now. Such state update could include properties like:
+		// * A `verticalSpacing` that has been measured in `onRender()`.
+		// * A cleaned-up `beforeResize` object that was cleaned-up in `onRender()`.
+		let stateUpdate = this._afterRenderStateUpdateThatWasStopped
+		this._afterRenderStateUpdateThatWasStopped = undefined
 
@@ -127,8 +126,6 @@ // Reset `this.verticalSpacing` so that it re-measures it in cases when
 				log('~ Scrollable container width changed from', prevWidth, 'to', newWidth, '~')
-				// `stateUpdate` doesn't get passed to `this.onResize()`, and, therefore,
-				// won't be applied. But that's ok because currently it could only contain:
-				// * `scrollableContainerWidth`
-				// * `verticalSpacing`
-				// * `beforeResize`
-				// All of those get rewritten in `onResize()` anyway.
+				// The pending state update (if present) won't be applied in this case.
+				// That's ok because such state update could currently only originate in
+				// `this.onResize()` function. Therefore, alling `this.onResize()` again
+				// would rewrite all those `stateUpdate` properties anyway, so they're not passed.
 				return this.onResize()
@@ -227,3 +224,5 @@ }
 	onItemHeightDidChange(i) {
-		this.hasToBeStarted()
+		// See the comments in the `setItemState()` function below for the rationale
+		// on why the `hasToBeStarted()` check was commented out.
+		// this.hasToBeStarted()
 		this._onItemHeightDidChange(i)
@@ -238,3 +237,44 @@ }
 	setItemState(i, newItemState) {
-		this.hasToBeStarted()
+		// There is an issue in React 18.2.0 when `useInsertionEffect()` doesn't run twice
+		// on mount unlike `useLayoutEffect()` in "strict" mode. That causes a bug in a React
+		// implementation of the `virtual-scroller`.
+		// https://gitlab.com/catamphetamine/virtual-scroller/-/issues/33
+		// https://github.com/facebook/react/issues/26320
+		// A workaround for that bug is ignoring the second-initial run of the effects at mount.
+		//
+		// But in that case, if an `ItemComponent` calls `setItemState()` in `useLayoutEffect()`,
+		// it could result in a bug.
+		//
+		// Consider a type of `useLayoutEffect()` that skips the initial mount:
+		// `useLayoutEffectSkipInitialMount()`.
+		// Suppose that effect is written in such a way that it only skips the first call of itself.
+		// In that case, if React is run in "strict" mode, the effect will no longer work as expected
+		// and it won't actually skip the initial mount and will be executed during the second initial run.
+		// But the `VirtualScroller` itself has already implemented a workaround that prevents
+		// its hooks from running twice on mount. This means that `useVirtualScrollerStartStop()`
+		// of the React component would have already stopped the `VirtualScroller` by the time
+		// `ItemComponent`'s incorrectly-behaving `useLayoutEffectSkipInitialMount()` effect is run,
+		// resulting in an error: "`VirtualScroller` hasn't been started".
+		//
+		// The log when not in "strict" mode would be:
+		//
+		// * `useLayoutEffect()` is run in `ItemComponent` — skips the initial run.
+		// * `useLayoutEffect()` is run in `useVirtualScrollerStartStop()`. It starts the `VirtualScroller`.
+		// * Some dependency property gets updated inside `ItemComponent`.
+		// * `useLayoutEffect()` is run in `ItemComponent` — no longer skips. Calls `setItemState()`.
+		// * The `VirtualScroller` is started so it handles `setState()` correctly.
+		//
+		// The log when in "strict" mode would be:
+		//
+		// * `useLayoutEffect()` is run in `ItemComponent` — skips the initial run.
+		// * `useLayoutEffect()` is run in `useVirtualScrollerStartStop()`. It starts the `VirtualScroller`.
+		// * `useLayoutEffect()` is unmounted in `useVirtualScrollerStartStop()`. It stops the `VirtualScroller`.
+		// * `useLayoutEffect()` is unmounted in `ItemComponent` — does nothing.
+		// * `useLayoutEffect()` is run the second time in `ItemComponent` — no longer skips. Calls `setItemState()`.
+		// * The `VirtualScroller` is stopped so it throws an error: "`VirtualScroller` hasn't been started".
+		//
+		// For that reason, the requirement of the `VirtualScroller` to be started was commented out.
+		// Commenting it out wouldn't result in any potential bugs because the code would work correctly
+		// in both cases.
+		// this.hasToBeStarted()
 		this._setItemState(i, newItemState)
@@ -241,0 +281,0 @@ }

source/VirtualScroller.layout.js

@@ -444,5 +444,9 @@ // For some weird reason, in Chrome, `setTimeout()` would lag up to a second (or more) behind.
 
-			// Update or reset a previously calculated layout
-			// so that the "diff"s based on that layout in the future
-			// produce correct results.
+			// Update or reset a previously calculated layout with the new item height
+			// so that the potential future "diff"s based on that "previously calculated" layout
+			// would be correct.
+			//
+			// The "previously calculated layout" feature is not currently used
+			// so this function call doesn't really affect anything.
+			//
 			updatePreviouslyCalculatedLayoutOnItemHeightChange.call(this, i, previousHeight, newHeight)
@@ -459,7 +463,9 @@
 			//
-			if (this.waitingForRender) {
-				log('~ Another state update is already waiting to be rendered. Delay the layout update until then. ~')
-				this.updateLayoutAfterRenderBecauseItemHeightChanged = true
-			} else {
-				this.onUpdateShownItemIndexes({ reason: LAYOUT_REASON.ITEM_HEIGHT_CHANGED })
+			if (this._isActive) {
+				if (this.waitingForRender) {
+					log('~ Another state update is already waiting to be rendered. Delay the layout update until then. ~')
+					this.updateLayoutAfterRenderBecauseItemHeightChanged = true
+				} else {
+					this.onUpdateShownItemIndexes({ reason: LAYOUT_REASON.ITEM_HEIGHT_CHANGED })
+				}
 			}
@@ -466,0 +472,0 @@

source/VirtualScroller.onRender.js

@@ -66,3 +66,3 @@ import log, { warn, reportError, isDebug } from './utility/debug.js'
 				warn('The most recent state that was set', getStateSnapshot(this.mostRecentSetStateValue))
-				reportError('The state that has been rendered is not the most recent one that was set')
+				reportError('`VirtualScroller` has been rendered with a `state` that is not equal to the most recently set one')
 			}
@@ -203,3 +203,3 @@ }
 		if (!this._isActive) {
-			this._stoppedStateUpdate = stateUpdate
+			this._afterRenderStateUpdateThatWasStopped = stateUpdate
 			return
@@ -206,0 +206,0 @@ }

No alert changes

Improved metrics

Total package byte prevSize

3622416

increased by2.32%

Number of package files

335

increased by6.69%

Lines of code

21860

increased by2.41%

No dependency changes