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

[9.x] Add Laravel-specific casting to the var dumper #44408

Closed
wants to merge 8 commits into from

Conversation

inxilpro
Copy link
Contributor

@inxilpro inxilpro commented Sep 30, 2022

I created laravel-dumper as a standalone package for three reasons: 1) I wanted to prove the concept, 2) I was concerned about proposing Laravel maintain its own VarDumper implementation, and 3) I wasn't sure if it was too opinionated for core.

It's now a successful package with ~300 stars and over 100,000 installs, so I think the "prove the concept" phase is done. And now that the new dd() improvements have been merged, the maintenance commitment concern has been addressed. That just leaves the "too opinionated" concern.

As a result, I've copied over much of the existing laravel-dumper implementation, but adjusted it in a few places to use more conservative defaults (and updated the implementation to match the existing Laravel core codebase).

As of now, I have custom casts for:

  • The service container
  • Database connections
  • Database builders
  • Carbon instances
  • HeaderBag and ParameterBag objects
  • Models
  • Requests and Responses

For the most part, these casts just simplify the output and remove noise (most notably in objects that hold a reference to the service container). In a few cases, I've also reordered attributes so that the data you're most likely wanting to view is at the top, or added virtual properties for easier debugging (like a nicely formatted date/time string for carbon instances).

You can see a full list of before/after diffs in the original package repo (although some diffs may be slightly different in this implementation—especially Models).

@dennisprudlo
Copy link
Contributor

No time to check the 1k diff so: Is this implemented as the new default or can one opt-in to that dumping style?

@inxilpro
Copy link
Contributor Author

inxilpro commented Oct 1, 2022

No time to check the 1k diff so: Is this implemented as the new default or can one opt-in to that dumping style?

This would be the new default. That said, each caster can be disabled individually, or you can opt to switch to the Symfony defaults with a few lines of code.

@inxilpro
Copy link
Contributor Author

inxilpro commented Oct 3, 2022

@nunomaduro what do you think of this?

@inxilpro inxilpro marked this pull request as ready for review October 5, 2022 15:02
@taylorotwell
Copy link
Member

I just still am hesitant to merge this in if it's already a maintained package. It shifts the burden to our team when we really can't take on a lot more burden at the moment. Would prefer to keep this a community effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants