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

fix(util): safely stringify path segments named as GROQ data types (e.g. null) #5986

Merged
merged 1 commit into from Mar 18, 2024

Conversation

juice49
Copy link
Contributor

@juice49 juice49 commented Mar 13, 2024

Description

Fields named as GROQ data types (true, false, and null) cannot be accessed using dot notation. These fields must instead be serialized using square bracket notation.

Segments names true, false, or null will now be serialized using square bracket notation. All other segments will continue to use dot notation.

Invalid

parent.true

Valid

parent["true"]

You can observe this by executing a GROQ query such as:

[{
  "true": "positive"
}, {
  "false": "negative"
}, {
  "null": "nothing"
}].null

// attribute expected

Versus:

[{
  "true": "positive"
}, {
  "false": "negative"
}, {
  "null": "nothing"
}]["null"]

// [
//   null,
//   null,
//   "nothing"
// ]

What to review

The paths created by the toString function are correct.

Testing

This branch adds tests to the packages/@sanity/util/test/PathUtils.test.ts test suite.

Copy link

vercel bot commented Mar 13, 2024

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

Name Status Preview Updated (UTC)
performance-studio ✅ Ready (Inspect) Visit Preview Mar 13, 2024 10:58am
test-studio ✅ Ready (Inspect) Visit Preview Mar 13, 2024 10:58am
1 Ignored Deployment
Name Status Preview Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Mar 13, 2024 10:58am

Copy link
Contributor Author

juice49 commented Mar 13, 2024

Copy link
Contributor

No changes to documentation

Copy link
Contributor

Component Testing Report Updated Mar 13, 2024 11:08 AM (UTC)

File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 32s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 6s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 12s 3 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 12s 4 2 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 13s 6 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 31s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 0s 15 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 1m 1s 18 0 0
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx ✅ Passed (Inspect) 12s 6 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 13s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 19s 9 0 0

@juice49 juice49 marked this pull request as ready for review March 13, 2024 11:24
@juice49 juice49 requested a review from a team as a code owner March 13, 2024 11:24
@juice49 juice49 requested review from bjoerge and removed request for a team March 13, 2024 11:24
Copy link
Member

@bjoerge bjoerge left a comment

Choose a reason for hiding this comment

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

Very nice, thank you!

Copy link
Contributor Author

juice49 commented Mar 18, 2024

Merge activity

  • Mar 18, 12:45 PM EDT: @juice49 started a stack merge that includes this pull request via Graphite.
  • Mar 18, 12:46 PM EDT: @juice49 merged this pull request with Graphite.

@juice49 juice49 merged commit 561ee14 into next Mar 18, 2024
37 checks passed
@juice49 juice49 deleted the fix/groq-data-type-path-access branch March 18, 2024 16:46
juice49 added a commit that referenced this pull request Mar 18, 2024
#5987)

### Description

When deriving search weights, we previously relied on a new `pathToString` function that replicated much of the functionality already implemented in the `toString` function from `@sanity/util/paths`. The one additional piece of functionality (handling segments that are `true`, `false`, or `null`) has now been implemented in #5986. Once merged, we can remove the redundant `pathToString` function and switch to the `toString` function from `@sanity/util/paths`.

### What to review

- Search requests are constructed correctly.

### Testing

There are tests for this functionality in `packages/sanity/src/core/search/common/__tests__/deriveSearchWeightsFromType.test.ts`.
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.

None yet

2 participants