beforeValidate and validate hooks race condition?
Not sure if this is a bug, but I'm seeing a race condition in this field definition. This fails validation every time, but if I put a console.log in the validate hook it passes every time... seems like
beforeValidateis not completing before
validatehook starts.
import slugify from "slugify";
const Slug = {
name: "slug",
type: "text",
localized: true,
required: true,
validate: (val) => {
return RegExp("^[a-z0-9]+(?:-[a-z0-9]+)*$").test(val)? true : "Invalid slug."
},
admin: {
position: "sidebar",
},
hooks: {
beforeValidate: [
({data, value}) => {
if (!value && data.title) {
return slugify(data.title)
}
}
]
}
};
export default Slug;
@alessiogr since you were so helpful in that other issue 😉
Adding
asyncto the beforeValidate hook seems to fix this but I cannot understand why...
{
name: "slug",
type: "text",
localized: true,
required: false,
validate: (val: string) => {
const isValid = RegExp("^$|[a-z0-9]+(?:-[a-z0-9]+)*$").test(val)? true : "Invalid slug."
return isValid;
},
admin: {
position: "sidebar",
},
hooks: {
beforeValidate: [({data, value}) => {
if (!value && data?.title) {
return slugify(data?.title)
}
}
]
}
},
required: false since you validate it, and allow empty to refresh the slug from title...
you can also do:
beforeValidate: [
({ data, value }) => {
if (!value && data?.title) {
return slugify(data?.title);
}
return slugify(value)
},
],
Hmm I don't know if that's it since adding a
console.logchanges the actual validation behavior, I think that really points to a race condition
There must be something I don't understand about the validate and beforeValidate lifecycle because it's not clear to me why adding
asyncwould change anything here.
on first glance, it does seem weird to me too
@ssyberg so the issue happens when you
firstcreate a new collection and put weird characters in the title, right?
Issue happens when I create a new record and put
anythingin the title
Slug stays empty and I get an invalid slug because it's empty
So for normal titles it works just fine for me. What I can consistently replicate is that it displays "Invalid slug" for weird titles like yxqchcey@∑€ƒ@∑€@ƒ
it's prob the same underlying issue so let's focus on that
https://github.com/AlessioGr/payload/blob/repro-beforevalidate/test/_community/collections/Slugged.tscreated a replication here so it's easier to debug and stalk through what the payload source is doing
https://github.com/AlessioGr/payload/blob/2379078c183089ddf3fa645df722b3a722a274bf/src/collections/operations/create.ts#L125this is probably the interesting part
validate: Provide a custom validation function that will be executed on both the Admin panel and the backend. there for you need to set required: false or else it never hits the validate.
It does hit the validate though (it displays "Invalid slug." as error message in the frontend and not that the field is a required value)
sorry beforeValidate on the server and the regex must allow empty: "^$|[a-z0-9]+(?:-[a-z0-9]+)
$" so it validates correctly.
{
name: "slug",
type: "text",
localized: true,
required: false,
validate: (val: string) => {
const isValid = RegExp("^$|[a-z0-9]+(?:-[a-z0-9]+)$").test(val)? true : "Invalid slug."
return isValid;
},
admin: {
position: "sidebar",
},
hooks: {
beforeValidate: [({data, value}) => {
if (!value && data?.title) {
return slugify(data?.title)
}
}
]
}
}
the regex itself is fine. The problem is that even wrong titles are not supposed to validate incorrectly (e.g. @/"") because the beforeValidate function is supposed to slugify the value and make it correct
before it validates
but it doesnt
but the client validate is preventing it to be sendt to the server 🙂
I think it's "beforeServerValidate"? when using the api?
hmhm that might just happen since it runs on the client too
this is weird
you'd expect beforeValidate to run before
what's the use-case of beforeValidated if that's the case though?
I think it's for when using the API
@Trstly the fact that a console.log added to the
validatefunction is pretty definitive evidence this is a race condition
you'd expect beforeValidate to run before
This to me is really the crux of it
Thing is, for incorrect tiles like
@/"")
even console.log is useless
I do think he is right about it
beforeValidateshould be
guaranteedto complete before
validateis called
I agree
on the other hand, if validate only ran on the server, you wouldnt get immediate feedback anymore
What I still dont understand is why adding console.log fixes it for you. I'd expect it to not work there as well
(if the order really is client validate => beforeValidate => server validate)
so somehow you're getting beforeValidate to run before client validate
client validate => beforeValidate => server validate
Oh do we have indication that is the lifecycle?
from what Trstly is saying it seems like that's the case. And it would kinda make sense
but not sure
Does it make sense? I think one of the primary use cases for
beforeValidateis setting a value for a field - which should then get validated (even on the client)
That's what I thought too! I didn't know it also runs on the client though
@jmikrut may I steal your time for a quick question for 15 seconds:
validate on client => beforeValidate on server => server validate on server
Is this lifecycle correct?
hello
yes
if i am understanding, the lifecycle does follow that path
Alright thank you!! Will make a PR to add it to the docs - I think it would be very helpful to clarify it there
@ssyberg what happens if you add something like
(if window || document) {
return true;
}
to the validate function?
to make sure it ONLY runs on the server
@jmikrut client validation takes place before the
beforeValidatehook?
Is there some other means of dynamically auto-populating a field before client validation?
client validation happens in the browser before the server and its hooks even come into the picture
yea i think alessio is right above, and that would be my suggestion too
but you could also use defaultValue
which
doesrun in the browser as well
does
defaultValuehave access to the document data?
As a way to just bypass client validation here? That might be the way
I guess the real solve here would be to add some client hooks or something so we could update that field on keyboard events like happens by default with the
title- I know we could potentially implement that with our own components, though that seems like a pretty big lift
Tagging this thread with a GH issue, the fact that a sleep/console log affects the behavior really still points to a race condition IMO:
https://github.com/payloadcms/payload/issues/2752I will review this shortly - I've been traveling this week but will respond on the GH issue shortly!
Continue the discussion in Discord
Star
Discord
online
Get help straight from the Payload team with an Enterprise License.