Skip to content

fix: validate "null" value for point field as true when its not required #12908

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

Merged
merged 2 commits into from
Jun 27, 2025

Conversation

dave-wwg
Copy link
Contributor

@dave-wwg dave-wwg commented Jun 23, 2025

What?

This PR solves an issue with validation of the point field in Payload CMS. If the value is null and the field is not required, the validation will return true before trying to examine the contents of the field

Why?

If the point field is given a value, and saved, it is then impossible to successfully "unset" the point field, either through the CMS UI or through a hook like beforeChange. Trying to do so will throw this error:

[17:09:41] ERROR: Cannot read properties of null (reading '0')
    err: {
      "type": "TypeError",
      "message": "Cannot read properties of null (reading '0')",
      "stack":
          TypeError: Cannot read properties of null (reading '0')
              at point (webpack-internal:///(rsc)/./node_modules/.pnpm/[email protected][email protected][email protected]/node_modules/payload/dist/fields/validations.js:622:40)

because a value of null will not be changed to the default value of ['',''], which in any case does not pass MongoDB validation either.

[17:22:49] ERROR: Cast to [Number] failed for value "[ NaN, NaN ]" (type string) at path "location.coordinates.0" because of "CastError"
    err: {
      "type": "CastError",
      "message": "Cast to [Number] failed for value \"[ NaN, NaN ]\" (type string) at path \"location.coordinates.0\" because of \"CastError\"",
      "stack":
          CastError: Cast to [Number] failed for value "[ NaN, NaN ]" (type string) at path "location.coordinates.0" because of "CastError"
              at SchemaArray.cast (webpack-internal:///(rsc)/./node_modules/.pnpm/[email protected]_@[email protected]/node_modules/mongoose/lib/schema/array.js:414:15)

How?

This adds a check to the top of the point validation function and returns early before trying to examine the contents of the point field

Copy link
Contributor

@DanRibbens DanRibbens left a comment

Choose a reason for hiding this comment

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

This looks great! I think we just need to see one int test before we can merge it.

I would add a test case for this one here

it('should create', async () => {

If you're unable to do this we can have somebody on our side work it into your PR. Let us know!

@dave-wwg
Copy link
Contributor Author

@DanRibbens ive added 2 integration tests, one each for required/non-required fields. I'm a little confused about this:

      const updatedDoc = await payload.update({
        collection: 'point-fields',
        data: {
          localized: null,
        },
        id: doc.id,
      })

      expect(updatedDoc.localized).toEqual(undefined)

where the updatedDoc's localized field is updated not to null but to undefined

hopefully its still alright! Let me know if there's any further changes to make. also totally fine if you/your team wants to push up a quick fix to get this merge-able

Copy link
Contributor

@DanRibbens DanRibbens left a comment

Choose a reason for hiding this comment

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

Great work!

EDIT: I didn't see your question. We don't handle null the way you're expecting?

We normalize the data in and out of the database so that null is undefined instead. This is expected.

@dave-wwg
Copy link
Contributor Author

gotcha, I wasn't aware of that. thanks for confirming it is expected

@DanRibbens DanRibbens enabled auto-merge (squash) June 26, 2025 22:33
@DanRibbens DanRibbens merged commit 2da6d92 into payloadcms:main Jun 27, 2025
152 of 154 checks passed
Copy link
Contributor

🚀 This is included in version v3.44.0

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.

2 participants