From 377f546c62614fe204be6d14c34abbbcdc26b0a9 Mon Sep 17 00:00:00 2001
From: Tim Kye <tim@kye.dev>
Date: Sun, 2 May 2021 11:36:10 -0700
Subject: [PATCH] default useLocationChange to skip initial render (#89)

* default useLocationChange to skip initial render

* update changelog

* clarify changelog fix for behavior reversion
---
 CHANGELOG.md                  |  5 +++++
 docs/api/useLocationChange.md |  3 +++
 docs/themes/learn             |  1 +
 src/hooks.js                  |  9 +++++++++
 src/path.js                   | 15 ++++++++++-----
 test/path.spec.js             | 16 ++++++++++++++--
 6 files changed, 42 insertions(+), 7 deletions(-)
 create mode 160000 docs/themes/learn
 create mode 100644 src/hooks.js

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ed328e0..72b219e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [2.1.0] - 2021-05-02
+### Added
+-  `options.onInitial` parameter for `useLocationChange` that controls the first render behavior. `default: false`.
+### Fixed
+- `useLocationChange` invoking the setter on initial render. This was not intended and was an unannounced change from the v1 behavior, so reverting it is not considered an API change but a bugfix.
 
 ## [2.0.2] - 2021-03-22
 ### Added
diff --git a/docs/api/useLocationChange.md b/docs/api/useLocationChange.md
index fba72a4..eb1c5c7 100644
--- a/docs/api/useLocationChange.md
+++ b/docs/api/useLocationChange.md
@@ -16,12 +16,15 @@ export function useLocationChange(
     inheritBasePath: boolean
     basePath: string
     isActive: () => boolean | boolean
+    onInitial?: boolean
   }
 ): void
 ```
 
 **Note**: `options.inheritBasePath` defaults to `true` (even if `options` is not provided), and takes precedence over `options.basePath` if `true`. If no BasePath is in the context to inherit `options.basePath` will be used as a fallback, if present. If `basePath` is provided, either by parameter or by context, and is missing from the current path `null` is sent to the `setFn` callback.
 
+By default this hook will not run on the initial mount for the component. You can get the location on the first render (mount) by setting `onInitial: true` in the `options` argument.
+
 ## Basic
 
 The first parameter is a setter-function that is invoked with the new path whenever the url is changed. It does not automatically cause a re-render of the parent component (see _re-rendering_ below).
diff --git a/docs/themes/learn b/docs/themes/learn
new file mode 160000
index 0000000..41c6cc5
--- /dev/null
+++ b/docs/themes/learn
@@ -0,0 +1 @@
+Subproject commit 41c6cc522f13ac324e99e8bf84f2b67d88510ceb
diff --git a/src/hooks.js b/src/hooks.js
new file mode 100644
index 0000000..7efae36
--- /dev/null
+++ b/src/hooks.js
@@ -0,0 +1,9 @@
+import { useLayoutEffect, useRef } from 'react'
+
+export function useMountedLayout(fn, deps, { onInitial = false } = {}) {
+  const hasMounted = useRef(onInitial)
+  useLayoutEffect(() => {
+    if (!hasMounted.current) hasMounted.current = true
+    else fn()
+  }, deps)
+}
diff --git a/src/path.js b/src/path.js
index 51bce41..63307fa 100644
--- a/src/path.js
+++ b/src/path.js
@@ -6,6 +6,7 @@ import {
   useLayoutEffect
 } from 'react'
 import { BasePathContext, PathContext } from './context.js'
+import { useMountedLayout } from './hooks.js'
 import { isNode, getSsrPath } from './node.js'
 import { isFunction } from './typeChecks.js'
 
@@ -62,7 +63,7 @@ export function getCurrentHash() {
 
 export function useLocationChange(
   setFn,
-  { inheritBasePath = true, basePath = '', isActive } = {}
+  { inheritBasePath = true, basePath = '', isActive, onInitial = false } = {}
 ) {
   if (isNode) return
   const routerBasePath = useBasePath()
@@ -92,10 +93,14 @@ export function useLocationChange(
 
   // When the basePath changes re-check the path after the render completes
   // This allows nested contexts to get an up-to-date formatted path
-  useLayoutEffect(() => {
-    if (isActive !== undefined && !isPredicateActive(isActive)) return
-    setRef.current(getFormattedPath(basePath))
-  }, [basePath, isActive])
+  useMountedLayout(
+    () => {
+      if (isActive !== undefined && !isPredicateActive(isActive)) return
+      setRef.current(getFormattedPath(basePath))
+    },
+    [basePath, isActive],
+    { onInitial }
+  )
 }
 
 /**
diff --git a/test/path.spec.js b/test/path.spec.js
index 456bec5..c042fc6 100644
--- a/test/path.spec.js
+++ b/test/path.spec.js
@@ -13,10 +13,22 @@ beforeEach(() => {
 })
 
 describe('useLocationChange', () => {
-  function Route({ onChange, isActive, basePath }) {
-    useLocationChange(onChange, { isActive, basePath })
+  function Route({ onChange, isActive, basePath, onInitial = false }) {
+    useLocationChange(onChange, { isActive, basePath, onInitial })
     return null
   }
+  test("setter doesn't get updated on mount", async () => {
+    let watcher = jest.fn()
+    render(<Route onChange={watcher} />)
+
+    expect(watcher).not.toBeCalled()
+  })
+  test('setter is updated on mount when onInitial is true', async () => {
+    let watcher = jest.fn()
+    render(<Route onChange={watcher} onInitial />)
+
+    expect(watcher).toBeCalled()
+  })
   test('setter gets updated path', async () => {
     let watcher = jest.fn()
     render(<Route onChange={watcher} />)