fix(tags): restore automatic hashtag creation for unknown tags

Two issues were preventing hashtags from being automatically added to the database:

1. **useTags.tsx - Stale closure bug**: The `addTag` function was using
   `tags` and `api` from the closure scope, which were stale when called.
   Fixed by using `useRef` for both `apiRef` and `tagsRef`, ensuring the
   function always accesses current values.

2. **PopupView.tsx - Tag creation in render**: New tags were detected
   inside the render `.map()` function with side effects - an anti-pattern.
   Moved tag processing to a dedicated `processItemsTags` function in
   useTags hook, called via useEffect when items are loaded.

Changes:
- Added `processItemsTags(items: Item[])` function to useTags hook
- Added `useProcessItemsTags` export hook
- Cleaned up PopupView.tsx to use the new hook via useEffect
- Removed side effects from render function

Now when items are loaded, any hashtags that don't exist in the database
are automatically created with a random color.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Anton Tranelis 2026-01-20 12:08:03 +01:00
parent 081f4f5476
commit d3e7b7a9bb
2 changed files with 82 additions and 34 deletions

View File

@ -1,6 +1,6 @@
/* eslint-disable @typescript-eslint/prefer-nullish-coalescing */
/* eslint-disable @typescript-eslint/restrict-plus-operands */
import { useContext, useMemo, useState } from 'react'
import { useContext, useEffect, useMemo } from 'react'
import { Marker, Tooltip } from 'react-leaflet'
import { useAppState } from '#components/AppShell/hooks/useAppState'
@ -13,18 +13,19 @@ import {
import { useItems, useAllItemsLoaded } from '#components/Map/hooks/useItems'
import { useAddMarker, useAddPopup, useLeafletRefs } from '#components/Map/hooks/useLeafletRefs'
import { useSetMarkerClicked, useSelectPosition } from '#components/Map/hooks/useSelectPosition'
import { useGetItemTags, useAllTagsLoaded, useTags } from '#components/Map/hooks/useTags'
import {
useGetItemTags,
useAllTagsLoaded,
useProcessItemsTags,
} from '#components/Map/hooks/useTags'
import LayerContext from '#components/Map/LayerContext'
import { ItemViewPopup } from '#components/Map/Subcomponents/ItemViewPopup'
import { encodeTag } from '#utils/FormatTags'
import { hashTagRegex } from '#utils/HashTagRegex'
import MarkerIconFactory from '#utils/MarkerIconFactory'
import { randomColor } from '#utils/RandomColor'
import TemplateItemContext from './TemplateItemContext'
import type { Item } from '#types/Item'
import type { Tag } from '#types/Tag'
import type { Popup } from 'leaflet'
/**
@ -51,9 +52,14 @@ export const PopupView = ({ children }: { children?: React.ReactNode }) => {
const setMarkerClicked = useSetMarkerClicked()
const selectPosition = useSelectPosition()
const tags = useTags()
const [newTagsToAdd, setNewTagsToAdd] = useState<Tag[]>([])
const [tagsReady, setTagsReady] = useState<boolean>(false)
const processItemsTags = useProcessItemsTags()
// Process items to create missing tags when items are loaded
useEffect(() => {
if (allTagsLoaded && allItemsLoaded && items.length > 0) {
processItemsTags(items)
}
}, [allTagsLoaded, allItemsLoaded, items, processItemsTags])
const isLayerVisible = useIsLayerVisible()
@ -105,24 +111,6 @@ export const PopupView = ({ children }: { children?: React.ReactNode }) => {
})
}
if (allTagsLoaded && allItemsLoaded) {
item.text?.match(hashTagRegex)?.map((tag) => {
if (
!tags.find((t) => t.name.toLocaleLowerCase() === tag.slice(1).toLocaleLowerCase()) &&
!newTagsToAdd.find((t) => t.name.toLocaleLowerCase() === tag.slice(1).toLocaleLowerCase())
) {
const newTag = {
id: crypto.randomUUID(),
name: tag.slice(1),
color: randomColor(),
}
setNewTagsToAdd((current) => [...current, newTag])
}
return null
})
!tagsReady && setTagsReady(true)
}
const itemTags = getItemTags(item)
const latitude = item.position.coordinates[1]

View File

@ -4,9 +4,10 @@
/* eslint-disable @typescript-eslint/prefer-optional-chain */
/* eslint-disable @typescript-eslint/no-non-null-assertion */
/* eslint-disable @typescript-eslint/no-unnecessary-condition */
import { useCallback, useReducer, createContext, useContext, useState } from 'react'
import { useCallback, useReducer, createContext, useContext, useState, useRef } from 'react'
import { hashTagRegex } from '#utils/HashTagRegex'
import { randomColor } from '#utils/RandomColor'
import type { Item } from '#types/Item'
import type { ItemsApi } from '#types/ItemsApi'
@ -22,6 +23,7 @@ const TagContext = createContext<UseTagManagerResult>({
setTagApi: () => {},
setTagData: () => {},
getItemTags: () => [],
processItemsTags: () => {},
allTagsLoaded: false,
})
@ -31,6 +33,7 @@ function useTagsManager(initialTags: Tag[]): {
setTagApi: (api: ItemsApi<Tag>) => void
setTagData: (data: Tag[]) => void
getItemTags: (item: Item) => Tag[]
processItemsTags: (items: Item[]) => void
allTagsLoaded: boolean
} {
const [allTagsLoaded, setallTagsLoaded] = useState<boolean>(false)
@ -53,10 +56,14 @@ function useTagsManager(initialTags: Tag[]): {
}
}, initialTags)
const [api, setApi] = useState<ItemsApi<Tag>>({} as ItemsApi<Tag>)
const apiRef = useRef<ItemsApi<Tag>>({} as ItemsApi<Tag>)
const tagsRef = useRef<Tag[]>(initialTags)
// Keep tagsRef in sync with tags state
tagsRef.current = tags
const setTagApi = useCallback(async (api: ItemsApi<Tag>) => {
setApi(api)
apiRef.current = api
const result = await api.getItems()
setTagCount(result.length)
if (tagCount === 0) setallTagsLoaded(true)
@ -79,15 +86,22 @@ function useTagsManager(initialTags: Tag[]): {
})
}, [])
const addTag = (tag: Tag) => {
const addTag = useCallback((tag: Tag) => {
// Check against current tags using ref to avoid stale closure
const tagExists = tagsRef.current.some(
(t) => t.name.toLocaleLowerCase() === tag.name.toLocaleLowerCase(),
)
dispatch({
type: 'ADD',
tag,
})
if (!tags.some((t) => t.name.toLocaleLowerCase() === tag.name.toLocaleLowerCase())) {
api.createItem && api.createItem(tag)
// Only create in API if tag doesn't already exist
if (!tagExists && apiRef.current.createItem) {
apiRef.current.createItem(tag)
}
}
}, [])
const getItemTags = useCallback(
(item: Item) => {
@ -117,7 +131,48 @@ function useTagsManager(initialTags: Tag[]): {
[tags],
)
return { tags, addTag, setTagApi, setTagData, getItemTags, allTagsLoaded }
// Process all items and create missing tags
const processItemsTags = useCallback(
(items: Item[]) => {
const currentTags = tagsRef.current
const newTags: Tag[] = []
items.forEach((item) => {
const text = item.text
text?.match(hashTagRegex)?.forEach((tag) => {
const tagName = tag.slice(1)
const tagNameLower = tagName.toLocaleLowerCase()
// Check if tag exists in current tags or already queued
const existsInTags = currentTags.some(
(t) => t.name.toLocaleLowerCase() === tagNameLower,
)
const existsInNew = newTags.some(
(t) => t.name.toLocaleLowerCase() === tagNameLower,
)
if (!existsInTags && !existsInNew) {
newTags.push({
id: crypto.randomUUID(),
name: tagName,
color: randomColor(),
})
}
})
})
// Add all new tags
newTags.forEach((tag) => {
dispatch({ type: 'ADD', tag })
if (apiRef.current.createItem) {
apiRef.current.createItem(tag)
}
})
},
[],
)
return { tags, addTag, setTagApi, setTagData, getItemTags, processItemsTags, allTagsLoaded }
}
export const TagsProvider: React.FunctionComponent<{
@ -156,3 +211,8 @@ export const useAllTagsLoaded = (): UseTagManagerResult['allTagsLoaded'] => {
const { allTagsLoaded } = useContext(TagContext)
return allTagsLoaded
}
export const useProcessItemsTags = (): UseTagManagerResult['processItemsTags'] => {
const { processItemsTags } = useContext(TagContext)
return processItemsTags
}