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

Local API: req.locale is mutated when performing queries #8922

Closed
mikkelwf opened this issue Oct 29, 2024 · 5 comments
Closed

Local API: req.locale is mutated when performing queries #8922

mikkelwf opened this issue Oct 29, 2024 · 5 comments
Labels

Comments

@mikkelwf
Copy link

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

@mikkelwf mikkelwf added status: needs-triage Possible bug which hasn't been reproduced yet v3 labels Oct 29, 2024
@jmikrut
Copy link
Member

jmikrut commented Oct 29, 2024

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. 👍

@GermanJablo
Copy link
Contributor

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!

@github-actions github-actions bot removed the status: needs-triage Possible bug which hasn't been reproduced yet label Nov 4, 2024
@mikkelwf
Copy link
Author

mikkelwf commented Nov 5, 2024

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.. :)

@GermanJablo
Copy link
Contributor

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!

Copy link

github-actions bot commented Nov 7, 2024

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants