Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix useMetadata leak #227

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Fix useMetadata leak #227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions ...ations/(evaluation)/[evaluationUuid]/editor/_components/EvaluationEditor/Editor/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
'use client'

import { Suspense, useCallback, useEffect, useMemo, useState } from 'react'
import { Suspense, useCallback, useMemo, useState } from 'react'

import { ConversationMetadata, readMetadata } from '@latitude-data/compiler'
import { promptConfigSchema } from '@latitude-data/core/browser'
import {
Button,
DocumentTextEditor,
DocumentTextEditorFallback,
} from '@latitude-data/web-ui'
import EditorHeader from '$/components/EditorHeader'
import { useMetadata } from '$/hooks/useMetadata'
import useEvaluations from '$/stores/evaluations'
import useProviderApiKeys from '$/stores/providerApiKeys'

Expand All @@ -30,11 +30,19 @@ export default function EvaluationEditor({
)
const { data: providers } = useProviderApiKeys()
const [value, setValue] = useState(defaultPrompt)
const [metadata, setMetadata] = useState<ConversationMetadata>()
const configSchema = useMemo(
() => promptConfigSchema({ providers: providers ?? [] }),
[providers],
)
const { metadata } = useMetadata(
{
prompt: value,
withParameters: EVALUATION_PARAMETERS,
configSchema,
},
[value, configSchema],
)

const save = useCallback(
(val: string) => {
update({
Expand All @@ -52,14 +60,6 @@ export default function EvaluationEditor({
[setValue],
)

useEffect(() => {
readMetadata({
prompt: value,
withParameters: EVALUATION_PARAMETERS,
configSchema,
}).then(setMetadata)
}, [value, configSchema])

if (!evaluation) return null

return (
Expand Down
35 changes: 13 additions & 22 deletions ...ersions/[commitUuid]/documents/[documentUuid]/_components/DocumentEditor/Editor/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,9 @@
'use client'

import path from 'path'
import {
createContext,
Suspense,
useCallback,
useEffect,
useMemo,
useState,
} from 'react'
import { createContext, Suspense, useCallback, useMemo, useState } from 'react'

import {
ConversationMetadata,
readMetadata,
Document as RefDocument,
} from '@latitude-data/compiler'
import { Document as RefDocument } from '@latitude-data/compiler'
import {
DocumentVersion,
promptConfigSchema,
Expand All @@ -28,6 +17,7 @@ import {
import { type AddMessagesActionFn } from '$/actions/sdk/addMessagesAction'
import type { RunDocumentActionFn } from '$/actions/sdk/runDocumentAction'
import EditorHeader from '$/components/EditorHeader'
import { useMetadata } from '$/hooks/useMetadata'
import useDocumentVersions from '$/stores/documentVersions'
import useProviderApiKeys from '$/stores/providerApiKeys'
import { useDebouncedCallback } from 'use-debounce'
Expand Down Expand Up @@ -68,11 +58,6 @@ export default function DocumentEditor({
)
const [value, setValue] = useState(document.content)
const [isSaved, setIsSaved] = useState(true)
const [metadata, setMetadata] = useState<ConversationMetadata>()
const configSchema = useMemo(
() => promptConfigSchema({ providers }),
[providers],
)

const debouncedSave = useDebouncedCallback(
(val: string) => {
Expand Down Expand Up @@ -128,14 +113,20 @@ export default function DocumentEditor({
[readDocumentContent, value],
)

useEffect(() => {
readMetadata({
const configSchema = useMemo(
() => promptConfigSchema({ providers }),
[providers],
)

const { metadata } = useMetadata(
{
prompt: value,
fullPath: document.path,
referenceFn: readDocument,
configSchema,
}).then(setMetadata)
}, [readDocument, configSchema])
},
[readDocument, configSchema],
)

const isMerged = commit.mergedAt !== null
return (
Expand Down
15 changes: 8 additions & 7 deletions .../evaluations/[evaluationId]/create-batch/_components/CreateBatchEvaluationModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
'use client'

import { useCallback, useEffect, useState } from 'react'
import { useCallback } from 'react'

import { ConversationMetadata, readMetadata } from '@latitude-data/compiler'
import { DocumentVersion, EvaluationDto } from '@latitude-data/core/browser'
import { Button, CloseTrigger, Modal } from '@latitude-data/web-ui'
import { useMetadata } from '$/hooks/useMetadata'
import { useNavigate } from '$/hooks/useNavigate'
import { ROUTES } from '$/services/routes'

Expand Down Expand Up @@ -42,13 +42,14 @@ export default function CreateBatchEvaluationModal({
goToDetail()
},
})
const [metadata, setMetadata] = useState<ConversationMetadata>()
useEffect(() => {
readMetadata({

const { metadata } = useMetadata(
{
prompt: document.content ?? '',
fullPath: document.path,
}).then(setMetadata)
}, [document])
},
[document],
)

const form = useRunBatchForm({ documentMetadata: metadata })
const onRunBatch = useCallback(() => {
Expand Down
20 changes: 20 additions & 0 deletions apps/web/src/hooks/useMetadata.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use client'

import { DependencyList, useEffect, useState } from 'react'

import { ConversationMetadata, readMetadata } from '@latitude-data/compiler'

type Props = Parameters<typeof readMetadata>[0]
export function useMetadata(props: Props, deps: DependencyList) {
csansoon marked this conversation as resolved.
Show resolved Hide resolved
const [isLoading, setIsLoading] = useState(true)
const [metadata, setMetadata] = useState<ConversationMetadata>()
useEffect(() => {
setIsLoading(true)
readMetadata(props).then((m) => {
setMetadata(m)
setIsLoading(false)
})
}, deps)

return { metadata, isLoading }
}
4 changes: 3 additions & 1 deletion apps/web/src/stores/providerApiKeys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { getProviderApiKeyAction } from '$/actions/providerApiKeys/fetch'
import useLatitudeAction from '$/hooks/useLatitudeAction'
import useSWR, { SWRConfiguration } from 'swr'

const EMPTY_ARRAY: ProviderApiKey[] = []

export default function useProviderApiKeys(opts?: SWRConfiguration) {
const { toast } = useToast()
const key = 'api/providerApiKeys'
Expand All @@ -24,7 +26,7 @@ export default function useProviderApiKeys(opts?: SWRConfiguration) {
return data || []
}
const {
data = [],
data = EMPTY_ARRAY,
Comment on lines -27 to +29
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guy was being was being set as a different "empty array" instance on every single render, and thus triggering anything that subscribed to it as a dependency. Thanks to this, opening a new document would compute the document's metadata 50+ times while this store was still loading its data.

mutate,
...rest
} = useSWR<ProviderApiKey[]>(key, fetcher, opts)
Expand Down
Loading