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

feat: proxy content type in controller factory #17772

Merged
merged 4 commits into from
Aug 28, 2023

Conversation

Marc-Roig
Copy link
Contributor

What does it do?

Some dynamically generated attributes (review workflows) were ignored in the core controller sanitizer. Thats because the content type metadata was loaded on startup rather than each time a request is made.

This PR solves the issue by proxying the content type attribute so every time someone get's its values it gets the latest content type data.

@Marc-Roig
Copy link
Contributor Author

Do you think it's better to use a proxy here, or a getter?

I feel the getter might have a different behaviour and this is a change that can impact custom generated controllers.

@Marc-Roig Marc-Roig added source: core:strapi Source is core/strapi package pr: fix This PR is fixing a bug labels Aug 24, 2023
@Marc-Roig Marc-Roig self-assigned this Aug 24, 2023
@@ -6,10 +6,20 @@ const { createController } = require('./core-api/controller');
const { createService } = require('./core-api/service');
const { createRoutes } = require('./core-api/routes');

// Content type is proxied to allow for dynamic content type updates
const getContentTypeProxy = (strapi, uid) => {
return new Proxy(strapi.contentType(uid), {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not really familiar with the proxy, could you explain a bit what's going on? (also why do we need the get(target, prop))

Copy link
Contributor Author

@Marc-Roig Marc-Roig Aug 24, 2023

Choose a reason for hiding this comment

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

a Proxy can wrap any javascript object, and intercept and redefine operations for that object.

With it you can extend how a get access behaves, and leave all the other operations as they were originally (set for example).
Whereas if we only define an object getter:

{
  get contentType() {
    return strapi.contentType(uid)
  }
}

we are only exposing the getter of this property, and nothing else.

the get(target, prop) is the interface the tthe proxy get exposes:

  • target : the original object you are proxying
  • prop: the property being accessed

@Marc-Roig Marc-Roig requested a review from Convly August 24, 2023 12:50
@Marc-Roig
Copy link
Contributor Author

I did some QA, I can not find anything that this changes breaks. Will wait for your review! :)

@Marc-Roig Marc-Roig merged commit 86a2a1f into main Aug 28, 2023
57 checks passed
@Marc-Roig Marc-Roig deleted the fix/assignee-private-field branch August 28, 2023 12:42
@Marc-Roig Marc-Roig added this to the 4.13.0 milestone Aug 30, 2023
@alexandrebodin alexandrebodin modified the milestones: 4.13.0, 4.13.1 Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fix This PR is fixing a bug source: core:strapi Source is core/strapi package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants