search

Stillat / blade-parser-typescript

A Laravel Blade parser, compiler, and static analyzer written in TypeScript.
https://stillat.com
MIT License
82 stars 2 forks source link

FR: support directives with multiple parameters #89

Closed claytonrcarter closed 7 months ago

claytonrcarter commented 7 months ago

Hi! I've been playing around w/ the prettier plugin and it's great. Thank you for it! I've noticed, however, that Blade directives with multiple parameters are not supported. There aren't many such directives, but @include (et al) and @each come to mind. In particular, we make heavy use of @include() to include subviews, and we almost always have to pass data into them. Here's an example adapted from the Laravel docs:

Example Input

@include( 'view.name'    ,     ['status' =>
'complete'])

Expected output

@include('view.name', ['status' => 'complete'])

Actual output (as of latest, v2.0.0)

unchanged

@include( 'view.name'    ,     ['status' =>
'complete'])

Possible solution

I hacked around a little bit and and – based on a thoroughly incomplete understanding of the codebase – found that the issue is happening in at least 2 places:

  1. hasValidPhp() is passing getPhpContent() to the validator, but in this case, that content looks like ( ... , ...) which is not valid PHP, because commas are not allowed there.
  2. Even if that's resolved, printDirective() is also using getPhpContent(), so it's also handling invalid PHP.

I see that there is already special handling for forelse in these places (eg in getContent() and printDirective()) so perhaps special handling needs to be added for include, etc?

As I said, I'm not familiar with the codebase, but I have been able to put together some failing tests for this, as well as hack together some code to get them to pass. But they are just hacks and I don't know how useful they would be for a proper fix. Would you like me to push them to a PR anyway, just to get the ball rolling?

JohnathonKoster commented 7 months ago

Thanks for the deep dive on this! I'd appreciate a PR to get the ball rolling and I can start diving in more later this week 💪

claytonrcarter commented 7 months ago

Happy to help! I've opened #90 with my fix for this, and #91 with some other issues I noticed.

JohnathonKoster commented 7 months ago

Available with release 2.1.0