payloadcms / payload

Payload is the open-source, fullstack Next.js framework, giving you instant backend superpowers. Get a full TypeScript backend and admin panel instantly. Use Payload as a headless CMS or for building powerful applications.
https://payloadcms.com
MIT License
24.75k stars 1.57k forks source link

Locale on `req` overwrites `locale` set on `findById` #4409

Closed RickGeersing closed 10 months ago

RickGeersing commented 10 months ago

Link to reproduction

No response

Describe the Bug

I've implemented an afterRead hook that fetches all urls of the same document with different locales. However if I pass the req and locale in the same request. It chooses the locale on req over the one provided on locale. I expect this to be the other way around.

My afterRead hook:

// findById request
const doc = await req.payload.findByID({
    collection: req.collection?.config?.slug as any,
    id: data.id,
    locale, // `en`
    req, // req.locale is set on `nl`
}) // Expect this to be in `en`

To Reproduce

  1. Create a findById call, and set it on a different locale that req.locale is
  2. You'll see that the document is fetched in the 'wrong' locale.

Payload Version

1.11.7

Adapters and Plugins

No response

RickGeersing commented 10 months ago

Haven't had the time to test this on 2.0, so could be fixed already. Not sure

RickGeersing commented 10 months ago

Problem was something completely else, changed the topic and contents.

DanRibbens commented 10 months ago

This is the line that sets the locale to use in the local findByID function: req.locale = locale ?? req?.locale ?? defaultLocale;

I'm guessing because you're passing the same req through many calls to payload.findByID and payload isn't making a copy of the req, it has all the same variables and so the locale may not be correct for each instance of it being called.

The best thing to do would be to isolate the req.locale property. You could add a function that dereferences the property on the req which you would call before the findByID function is called.

export default function isolateProperty(req: PayloadRequest, property: string): PayloadRequest {
  const delegate = {}
  const handler: ProxyHandler<PayloadRequest> = {
    deleteProperty(target, p): boolean {
      return Reflect.deleteProperty(p === property ? delegate : target, p)
    },
    get(target, p, receiver) {
      return Reflect.get(p === property ? delegate : target, p, receiver)
    },
    has(target, p) {
      return Reflect.has(p === property ? delegate : target, p)
    },
    set(target, p, newValue, receiver) {
      if (p === property) {
        // in case of transactionID we must ignore any receiver, because
        // "If provided and target does not have a setter for propertyKey, the property will be set on receiver instead."
        return Reflect.set(delegate, p, newValue)
      } else {
        return Reflect.set(target, p, newValue, receiver)
      }
    },
  }
  return new Proxy(req, handler)
}

Called with: isolateProperty(req, 'locale')

I'm pretty sure this will solve your issue and it is something we should consider a fix for the current version. Before we do anything we need to know this is the actual cause and include a reproduction so we can properly diagnose.

JarrodMFlesch commented 10 months ago

@RickGeersing can you provide a reproduction? I threw together this example collection and it worked as I expected, prioritizing en over es which was defined on the request

{
  slug: 'test',
  access: {
    read: () => true,
  },
  hooks: {
    afterRead: [
      async ({ req, doc }) => {
        if (req?.context?.test) return
        const test = await req.payload.findByID({
          collection: req.collection?.config?.slug as any,
          id: doc.id,
          locale: 'en', // `en`
          req, // req.locale is set on `es`
          context: {
            test: 'test',
          },
          depth: 0,
        })

        console.log(test)
      },
    ],
  },
  fields: [
    {
      name: 'title',
      type: 'text',
      localized: true,
    },
  ],
},

The request url looked like this:

http://localhost:3000/api/test?locale=es

The response consistently returns the english version of the document.

RickGeersing commented 10 months ago

In the meantime we've upgraded to 2.0, that seems to have fixed it.

JarrodMFlesch commented 10 months ago

Ok marking as resolved since it is fixed on latest. Thank you for confirming.

github-actions[bot] commented 1 month ago

This issue has been automatically locked. Please open a new issue if this issue persists with any additional detail.