Skip to content

feat: introduce transformRelated() for smart and isolated relationship transformations#10278

Draft
datamweb wants to merge 5 commits into
codeigniter4:4.8from
datamweb:feat/api-transformer-relations
Draft

feat: introduce transformRelated() for smart and isolated relationship transformations#10278
datamweb wants to merge 5 commits into
codeigniter4:4.8from
datamweb:feat/api-transformer-relations

Conversation

@datamweb
Copy link
Copy Markdown
Contributor

@datamweb datamweb commented Jun 5, 2026

Description
This PR introduces a new method transformRelated(), to the BaseTransformer class to solve the Global State Leakage issue when transforming nested/related resources, and to simplify the developer experience.

Previously, the documentation instructed developers to manually instantiate transformers for included resources:

return (new PostTransformer())->transformMany($posts);

This approach caused a significant issue: if a request contained query parameters like ?include= or ?fields=, those global states could unintentionally leak into the child transformers, leading to incorrect field filtering, infinite loops, or ApiExceptions.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@github-actions github-actions Bot added the 4.8 PRs that target the `4.8` branch. label Jun 5, 2026
@datamweb datamweb marked this pull request as draft June 5, 2026 10:58
@michalsn
Copy link
Copy Markdown
Member

michalsn commented Jun 5, 2026

This approach caused a significant issue: if a request contained query parameters like ?include= or ?fields=, those global states could unintentionally leak into the child transformers, leading to incorrect field filtering, infinite loops, or ApiExceptions.

Shouldn't the behavior of transformMany() be treated as a bug and fixed?

@datamweb
Copy link
Copy Markdown
Contributor Author

datamweb commented Jun 5, 2026

Shouldn't the behavior of transformMany() be treated as a bug and fixed?

@michalsn The issue actually isn't inside transformMany(). The leakage happens the moment we instantiate the new transformer.

If the URL is /users?include=comments:

// The leak happens right here, before transformMany() is even called!
$transformer = new CommentTransformer(); 

Because we don’t pass a custom $request to the constructor, it automatically grabs the global Services::request(). This global request still contains include=comments, which infects the child transformer.
If we change how BaseTransformer or transformMany() handles the global request, we might break existing applications that accidentally or intentionally rely on this behavior.
And transformRelated() doesn’t just fix the leak. It also saves developers from checks to decide whether to call transform() (for single items) or transformMany() (for collections).

@datamweb
Copy link
Copy Markdown
Contributor Author

datamweb commented Jun 5, 2026

On a related note, one of the main reasons I was so keen on introducing transformRelated() to isolate the state is that it lays the perfect architectural groundwork for an advanced feature I'd love to see in the future, Scoped Field Filtering.

Right now, if we want to support advanced API queries like:

/users?include=comments,posts&fields[posts]=id,slug

Handling this with the old architecture would have been a nightmare because the global state leakage made child transformers unpredictable. However, now that transformRelated() provides a clean, isolated boundary for child instantiation, extending this method in the future to parse and pass targeted scopes/fields directly to the child transformer will be incredibly straightforward and safe.

@datamweb datamweb marked this pull request as ready for review June 5, 2026 12:50
@datamweb datamweb closed this Jun 5, 2026
@datamweb datamweb reopened this Jun 5, 2026
Copy link
Copy Markdown
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

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

Okay, I looked into the actual code. While this new method can be really handy as API sugar, which we can promote as the default method to use, this doesn't really fix the underlying problem.

I will try to prepare something as a bugfix targeting the develop branch.

@datamweb datamweb marked this pull request as draft June 6, 2026 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.8 PRs that target the `4.8` branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants