payloadcms/payload

Local API: req.locale is mutated when performing queries

Closed this issue ยท 5 comments

Describe the Bug

I'm trying to use the beforeSync transformer in the search plugin to alter collection data prior to saving it to search results.
I use localized fields. When i want to utilize req.locale to see which data I need to change, the locale has been changed from the selected locale (en or da) to all.

It's because the syncWithSearch hook from the search plugin set the locale to all and this call also has the req object included in the config.

The problems is not only with search, as far as I can see all calls to the locale api that includes the req object, like

async ({req}) => {
   console.log(req.locale) // 'en'
   await payload.find({collection: 'foo', locale: 'da', req})
   console.log(req.locale) // 'da'
   await payload.find({collection: 'foo', locale: 'all', req})
   console.log(req.locale) // 'all'
}

I can omit including the req, but then I opt out of transactions (AFAIK).

Also mutating the req object seems strange, shouldn't this be kinda static?

Link to the code that reproduces this issue

https://github.com/re-cph/payload/tree/bug/req_locale_mutation

Reproduction Steps

Run tests

Which area(s) are affected? (Select all that apply)

plugin: search, db-sqlite, area: core

Environment Info

Payload: 3.0.0-beta.120
Nodejs: 20.10.0
next: 15.0.1

Hey @mikkelwf this is good detective work. As things are now, the locale can be mutated from query to query and it is not actually static as it might seem like it should be. I definitely think it's misleading.

It has been on the backburner for us to change how this works for a while now. I actually don't think we should have the locale tied to the req whatsoever. Instead, it should be arguments to hooks / access control / etc.

Now might be the time to make this change. Thanks for bringing this up. ๐Ÿ‘

Hi @mikkelwf . Thanks to this PR this issue should be resolved as of v3.0.0-beta.121, as search-plugin no longer mutates the req.

Thank you very much for reporting the bug!

Thanks! I've tested the search plugin and it works as expected now :)

I still think you should consider changing the mutating aspect of the locale included inside re.
I was quite baffled about the locale change, and its might not be apparent for consumers of that api, that the locale can be changed in the background by accident.

But its probably a post 3.0 release thing to fix.. :)

That was exactly our conclusion.
Ideally req should not be mutable, but that change cannot happen in the near future because it would be a breaking change. Maybe later!

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