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.

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

refactor: consistently apply current perspective for previews #8655

Open
wants to merge 5 commits into
base: next
Choose a base branch
from

Conversation

bjoerge
Copy link
Member

@bjoerge bjoerge commented Feb 17, 2025

Description

This is a somewhat big refactor of how we resolve values for previews internally.

Previously, for each document the studio was previewing, we'd fetch a version of the preview for each known release.
This created some overhead (althought the requests are batched), but most notably it caused an issue with previews ending up showing referenced values from the published version.

This is now solved by always applying the current perspective stack when fetching preview values. The perspective stack is applied recursively, so if a document type has someReference.title in it's preview config, the title of the referenced document will also reflect the current perspective (i.e. what the title is in the selected release).

Making the perspective apply to previews enabled me to remove some layering logic, since this is now done in the query that fetches the previews.

Using the just the current perspective stack poses a challenge: Sometimes you want to preview something that does not exist in the current perspective. For example: you are in the drafts perspective, but you want to preview a document that only exists as a version (i.e. it has neither a draft nor a published variant). This happens e.g. when previewing document list items or search results since we these collections using perspective=raw because we want people to still find version documents that exists outside of the current perspective.

For these cases, we always have a version id, so in these cases we'll fetch:

  • A preview for the document with current perspective applied
  • A preview for the document with the version perspective applied.
    If the document doesn’t exist in the current perspective, it certainly exist in it’s own version perspective, so that means we’ll always have something to preview.

Things to note:

  • Since perspective applies to the query request, I had to change the preview observer batch fetcher memo to include the perspective stack as key.
  • The previous implementation fetched a preview version for each known release in order to answer "what other releases does this document exist in". This question can be answered more simply by fetching the _rev for each version document and check for existence (so no need to fetch the full preview). The information about what other version exists is now decoupled from previews entirely, and can be retrieved using the useDocumentVersionInfo()-hook. We no longer have preview for the version documents because we didn't have any use for that at the moment. We can always come back to that decision in the future, but for now, they return a stub with _id, _createdAt, _updatedAt, _rev, etc.
  • We were currently supporting a perspective for search, which only seems to apply to the global search atm. I took a chance on removing that, because I don't think that's desired. As of this PR, global search will use perspective=raw.

Other changes

  • Added a
  • Simplified the getPreviewStateObservable() so it no longer requires a fallback title – the fallback can trivially be applied in the consumer end, and it lead to a bunch of cases where we passed an empty string as the third argument.

What to review

This should be reviewed a bit carefully. Esp. in terms of API changes and naming (is it sensible and understandable?)

Testing

We have fairly good test coverage already, and e2e tests already made me aware of a few oversights. Would love some additional testing of everything related to previews, search and references e.g:

  • Document list previews
  • Global search
  • Adding documents to release
  • Searching document lists
  • Searching for references
  • Reference previews in Portable Text Editor, arrays and objects

Notes for release

  • Fixes a bug that caused previews to sometimes display values from published documents instead of drafts or versions.
  • Fixes a bug that caused documents outside of the current release to not appear in global search
  • Fixes a bug that would show "Missing document" for any document outside of the selected release without published or draft variants.

Copy link

vercel bot commented Feb 17, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
page-building-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 20, 2025 4:46pm
performance-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 20, 2025 4:46pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 20, 2025 4:46pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Feb 20, 2025 4:46pm
test-next-studio ⬜️ Ignored (Inspect) Feb 20, 2025 4:46pm

Copy link
Contributor

No changes to documentation

Copy link
Contributor

github-actions bot commented Feb 19, 2025

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 43.11% 55111 / 127816
🔵 Statements 43.11% 55111 / 127816
🔵 Functions 48.28% 2806 / 5811
🔵 Branches 79.59% 10693 / 13435
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/sanity/src/core/index.ts 100% 100% 100% 100%
packages/sanity/src/core/comments/hooks/useNotificationTarget.ts 24.24% 100% 0% 24.24% 32-61
packages/sanity/src/core/components/documentStatus/DocumentStatus.tsx 20.63% 100% 0% 20.63% 33-79, 101-117
packages/sanity/src/core/components/documentStatusIndicator/DocumentStatusIndicator.tsx 15.68% 100% 0% 15.68% 48-96
packages/sanity/src/core/form/inputs/ReferenceInput/OptionPreview.tsx 13.95% 100% 0% 13.95% 24-83
packages/sanity/src/core/form/inputs/ReferenceInput/PreviewReferenceValue.tsx 6.62% 100% 0% 6.62% 24-235
packages/sanity/src/core/form/inputs/ReferenceInput/ReferenceField.tsx 10.81% 100% 0% 10.81% 43-59, 63-310
packages/sanity/src/core/form/inputs/ReferenceInput/ReferenceInput.tsx 11.02% 100% 0% 11.02% 37-39, 46-373
packages/sanity/src/core/form/inputs/ReferenceInput/ReferenceItem.tsx 9.06% 100% 0% 9.06% 54-70, 74-406
packages/sanity/src/core/form/inputs/ReferenceInput/ReferencePreview.tsx 16.66% 100% 0% 16.66% 23-78
packages/sanity/src/core/form/inputs/ReferenceInput/types.ts 0% 0% 0% 0%
packages/sanity/src/core/form/inputs/ReferenceInput/useReferenceInput.tsx 12.5% 100% 0% 12.5% 23-28, 39-137
packages/sanity/src/core/form/studio/inputs/client-adapters/reference.ts 10.98% 100% 0% 10.98% 31-179, 182-234
packages/sanity/src/core/preview/createPathObserver.ts 49.5% 52.94% 83.33% 49.5% 11-16, 24-25, 45-46, 53-59, 70, 75-89, 95-115
packages/sanity/src/core/preview/createPreviewObserver.ts 40% 100% 75% 40% 28-30, 55-76, 80-101, 104-110
packages/sanity/src/core/preview/documentPreviewStore.ts 83.05% 100% 33.33% 83.05% 146-154, 179
packages/sanity/src/core/preview/observeFields.ts 80.58% 100% 66.66% 80.58% 145, 150, 159-162, 178-184, 197, 214, 248-255, 258-267
packages/sanity/src/core/preview/types.ts 0% 0% 0% 0%
packages/sanity/src/core/preview/useValuePreview.ts 94.11% 66.66% 66.66% 94.11% 56-57
packages/sanity/src/core/preview/utils/getPreviewStateObservable.ts 87.87% 33.33% 100% 87.87% 48, 52-54
packages/sanity/src/core/preview/utils/getPreviewValueWithFallback.tsx 16.66% 100% 0% 16.66% 6-15, 24-38
packages/sanity/src/core/releases/store/index.ts 100% 100% 100% 100%
packages/sanity/src/core/releases/store/types.ts 100% 100% 100% 100%
packages/sanity/src/core/releases/store/useDocumentVersionInfo.ts 81.63% 100% 50% 81.63% 13-15, 41-46
packages/sanity/src/core/releases/tool/components/ReleaseDocumentPreview.tsx 97.91% 85% 100% 97.91% 69
packages/sanity/src/core/releases/tool/detail/AddDocumentSearch.tsx 68.08% 100% 50% 68.08% 33-53, 60
packages/sanity/src/core/releases/tool/detail/ReleaseReview.tsx 16.88% 100% 0% 16.88% 43-121
packages/sanity/src/core/releases/tool/detail/ReleaseSummary.tsx 98.66% 90.9% 50% 98.66% 76
packages/sanity/src/core/releases/tool/detail/useBundleDocuments.ts 8.91% 100% 0% 8.91% 53-157, 160-218, 221-260, 266-284
packages/sanity/src/core/releases/tool/detail/documentTable/DocumentTableColumnDefs.tsx 84.9% 82.75% 100% 84.9% 69-74, 185-205
packages/sanity/src/core/releases/tool/detail/review/DocumentReviewHeader.tsx 15.62% 100% 0% 15.62% 37-104
packages/sanity/src/core/scheduledPublishing/components/scheduleItem/PreviewWrapper.tsx 15.32% 100% 0% 15.32% 45-203, 209-211
packages/sanity/src/core/store/_legacy/document/getPairListener.ts 76.1% 96.66% 75% 76.1% 59-72, 86-87, 121, 136-137, 140-153, 176, 185, 192-202
packages/sanity/src/core/studio/components/navbar/search/components/filters/common/ReferencePreviewTitle.tsx 35.71% 100% 0% 35.71% 16-32
packages/sanity/src/core/studio/components/navbar/search/components/searchResults/item/SearchResultItem.tsx 16.04% 100% 0% 16.04% 39-123
packages/sanity/src/core/studio/components/navbar/search/components/searchResults/item/SearchResultItemPreview.tsx 20% 100% 0% 20% 48-114
packages/sanity/src/core/studio/components/navbar/search/contexts/search/SearchProvider.tsx 92.9% 50% 25% 92.9% 114-119, 148, 171-172, 200-201
packages/sanity/src/core/studio/components/navbar/search/contexts/search/reducer.ts 88.88% 68.42% 100% 88.88% 163-164, 174-177, 234-242, 244-251, 361-368, 393-399, 487-497, 570
packages/sanity/src/core/tasks/hooks/useDocumentPreviewValues.ts 22.58% 100% 0% 22.58% 22-50
packages/sanity/src/presentation/editor/ContentEditor.tsx 0% 0% 0% 0% 1-156
packages/sanity/src/presentation/editor/usePreviewState.ts 0% 0% 0% 0% 1-37
packages/sanity/src/structure/components/paneItem/PaneItemPreview.tsx 10% 100% 0% 10% 38-98
packages/sanity/src/structure/panes/documentList/sheetList/useDocumentSheetColumns.tsx 97.77% 85% 100% 97.77% 103, 136-137
Generated in workflow #30792 for commit e13cf83 by the Vitest Coverage Report Action

Copy link
Contributor

github-actions bot commented Feb 19, 2025

⚡️ Editor Performance Report

Updated Thu, 20 Feb 2025 16:51:56 GMT

Benchmark reference
latency of sanity@latest
experiment
latency of this branch
Δ (%)
latency difference
article (title) 24.4 efps (41ms) 23.8 efps (42ms) +1ms (+2.4%)
article (body) 69.9 efps (14ms) 65.6 efps (15ms) +1ms (-/-%)
article (string inside object) 27.8 efps (36ms) 24.7 efps (41ms) +5ms (+12.5%)
article (string inside array) 24.4 efps (41ms) 22.7 efps (44ms) +3ms (+7.3%)
recipe (name) 45.5 efps (22ms) 45.5 efps (22ms) +0ms (-/-%)
recipe (description) 55.6 efps (18ms) 47.6 efps (21ms) +3ms (+16.7%)
recipe (instructions) 99.9+ efps (6ms) 99.9+ efps (5ms) -1ms (-/-%)

efps — editor "frames per second". The number of updates assumed to be possible within a second.

Derived from input latency. efps = 1000 / input_latency

Detailed information

🏠 Reference result

The performance result of sanity@latest

Benchmark latency p75 p90 p99 blocking time test duration
article (title) 41ms 46ms 50ms 154ms 197ms 10.6s
article (body) 14ms 18ms 28ms 178ms 204ms 5.5s
article (string inside object) 36ms 39ms 43ms 308ms 340ms 7.1s
article (string inside array) 41ms 44ms 49ms 166ms 269ms 7.1s
recipe (name) 22ms 25ms 32ms 51ms 0ms 7.9s
recipe (description) 18ms 19ms 21ms 34ms 0ms 4.5s
recipe (instructions) 6ms 7ms 9ms 27ms 0ms 3.2s

🧪 Experiment result

The performance result of this branch

Benchmark latency p75 p90 p99 blocking time test duration
article (title) 42ms 56ms 82ms 526ms 1016ms 11.9s
article (body) 15ms 17ms 28ms 179ms 251ms 5.8s
article (string inside object) 41ms 43ms 48ms 65ms 118ms 6.9s
article (string inside array) 44ms 46ms 51ms 236ms 250ms 7.2s
recipe (name) 22ms 24ms 27ms 56ms 1ms 7.6s
recipe (description) 21ms 22ms 23ms 69ms 8ms 5.1s
recipe (instructions) 5ms 7ms 8ms 26ms 0ms 3.3s

📚 Glossary

column definitions

  • benchmark — the name of the test, e.g. "article", followed by the label of the field being measured, e.g. "(title)".
  • latency — the time between when a key was pressed and when it was rendered. derived from a set of samples. the median (p50) is shown to show the most common latency.
  • p75 — the 75th percentile of the input latency in the test run. 75% of the sampled inputs in this benchmark were processed faster than this value. this provides insight into the upper range of typical performance.
  • p90 — the 90th percentile of the input latency in the test run. 90% of the sampled inputs were faster than this. this metric helps identify slower interactions that occurred less frequently during the benchmark.
  • p99 — the 99th percentile of the input latency in the test run. only 1% of sampled inputs were slower than this. this represents the worst-case scenarios encountered during the benchmark, useful for identifying potential performance outliers.
  • blocking time — the total time during which the main thread was blocked, preventing user input and UI updates. this metric helps identify performance bottlenecks that may cause the interface to feel unresponsive.
  • test duration — how long the test run took to complete.

Copy link
Contributor

github-actions bot commented Feb 19, 2025

Component Testing Report Updated Feb 20, 2025 4:54 PM (UTC)

❌ Failed Tests (6) -- expand for details
File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 1m 22s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 14s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ❌ Failed (Inspect) 2m 41s 3 0 3
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 59s 11 7 0
formBuilder/inputs/PortableText/copyPaste/CopyPasteFields.spec.tsx ✅ Passed (Inspect) 0s 0 12 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 30s 6 0 0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx ✅ Passed (Inspect) 17s 3 0 0
formBuilder/inputs/PortableText/DragAndDrop.spec.tsx ✅ Passed (Inspect) 31s 6 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ❌ Failed (Inspect) 1m 56s 14 0 1
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 50s 21 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ❌ Failed (Inspect) 2m 59s 20 0 1
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx ✅ Passed (Inspect) 15s 3 9 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 31s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ❌ Failed (Inspect) 2m 40s 20 0 1
formBuilder/tree-editing/TreeEditing.spec.tsx ✅ Passed (Inspect) 0s 0 3 0
formBuilder/tree-editing/TreeEditingNestedObjects.spec.tsx ✅ Passed (Inspect) 0s 0 3 0

@bjoerge bjoerge force-pushed the sapp-2116 branch 2 times, most recently from 63ab336 to 0fbe326 Compare February 19, 2025 18:51
@bjoerge bjoerge marked this pull request as ready for review February 19, 2025 22:00
@bjoerge bjoerge requested review from a team as code owners February 19, 2025 22:00
@bjoerge bjoerge requested review from jordanl17 and removed request for a team February 19, 2025 22:00
@bjoerge bjoerge marked this pull request as draft February 19, 2025 22:01
@bjoerge bjoerge marked this pull request as ready for review February 20, 2025 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant