From 70428228a5a52e5655c1b7fd2ac57762fa3087bf Mon Sep 17 00:00:00 2001
From: David Joy <davidjoy@users.noreply.github.com>
Date: Wed, 1 Apr 2020 16:07:58 -0400
Subject: [PATCH] fix: Fix UserMessagesProvider state references (#38)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

See details in code, but this causes UserMessagesProvider to always use the most “recent” version of its messages and nextId state when its callbacks are called.
---
 src/user-messages/UserMessagesProvider.jsx | 32 ++++++++++++++--------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/src/user-messages/UserMessagesProvider.jsx b/src/user-messages/UserMessagesProvider.jsx
index 56f0d835..2f72bc9d 100644
--- a/src/user-messages/UserMessagesProvider.jsx
+++ b/src/user-messages/UserMessagesProvider.jsx
@@ -4,32 +4,40 @@ import PropTypes from 'prop-types';
 import UserMessagesContext from './UserMessagesContext';
 
 export default function UserMessagesProvider({ children }) {
+  // Note: The callbacks (add, remove, clear) below interact with useState in very subtle ways.
+  // When we call setMessages, we always do so with the function-based form of the handler, making
+  // use of the "current" state and not relying on lexical scoping to access the state exposed
+  // above with useState.  This is very important and allows us to call multiple "add", "remove",
+  // or "clear" functions in a  single render.  Without it, each call to one of the callbacks
+  // references back to the -original- value of messages instead of the most recent, causing them
+  // all to override each other.  Last one in would win.
   const [messages, setMessages] = useState([]);
   const [nextId, setNextId] = useState(1);
 
-  const refMessages = useRef(messages);
+  // Because the add, remove, and clear handlers also need to access nextId, we have to do
+  // something a bit different.  There's no way to wait for the "currentNextId" in a setMessages
+  // handler.  The alternative is to update a ref, which will always point to the current value by
+  // its very nature.
+  const refId = useRef(nextId);
 
   const add = ({
     code, dismissible, text, type, topic, ...others
   }) => {
-    const id = nextId;
-    refMessages.current = [...refMessages.current, {
+    const id = refId.current;
+    setMessages(currentMessages => [...currentMessages, {
       code, dismissible, text, type, topic, ...others, id,
-    }];
-    setMessages(refMessages.current);
-    setNextId(nextId + 1);
-    return id;
+    }]);
+    refId.current += 1;
+    setNextId(refId.current);
+    return refId.current;
   };
 
   const remove = id => {
-    refMessages.current = refMessages.current.filter(message => message.id !== id);
-    setMessages(refMessages.current);
+    setMessages(currentMessages => currentMessages.current.filter(message => message.id !== id));
   };
 
   const clear = (topic = null) => {
-    refMessages.current = topic === null ? [] : refMessages.current.filter(message => message.topic !== topic);
-
-    setMessages(refMessages.current);
+    setMessages(currentMessages => (topic === null ? [] : currentMessages.filter(message => message.topic !== topic)));
   };
 
   const value = {