Stillat / blade-parser-typescript

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

Formatting issues #63

Closed joedixon closed 1 year ago

joedixon commented 1 year ago

Hey @JohnathonKoster 👋

First of all, loving your work on this plugin, it's very cool to see how much progress you're making.

Just wanted to highlight a couple of minor issues I've run into. The first is related to conditional includes which you can see below:

Screenshot 2023-06-21 at 13 05 01

Second, an issue with escaping blade tags for use with Javascript.

Screenshot 2023-06-21 at 12 03 48

If there is any further info I can provide to help here, please let me know.

JohnathonKoster commented 1 year ago

Thanks for the report! I'll look into both of these (the escaped one might be challenging since I'm not guaranteed to always have the }} closing sequence when we start escaped content).

For the conditional include, is the screenshot the before or after results? Having the before/after code samples as text really helps, as well as your prettier configuration (I am unable to reproduce either on my end).

Thank you!

joedixon commented 1 year ago

Yeah, that does sound tricky. I guess in most cases you would expect a closing }}, but definitely can't rely on it.

Here's the full diff of the conditional includes:

Screenshot 2023-06-21 at 12 02 59

JohnathonKoster commented 1 year ago

Would you be able to provide your package.json, the original (or comparable) file that is experiencing issues, as well as the prettier configuration?

I have been unable to reproduce, and having that information helps immensely.

joedixon commented 1 year ago

Sure, here is the package.json

{
  "private": true,
  "scripts": {
    "dev": "npm run development",
    "development": "mix",
    "watch": "mix watch",
    "watch-poll": "mix watch -- --watch-options-poll=1000",
    "hot": "mix watch --hot",
    "production": "mix --production",
    "prod": "npm run production"
  },
  "devDependencies": {
    "@fullhuman/postcss-purgecss": "^5.0",
    "axios": "^1.0",
    "bootstrap": "^5.0",
    "brace": "^0.11.0",
    "cross-env": "^5.1",
    "jquery": "^3.7.0",
    "laravel-echo": "^1.8.1",
    "laravel-mix": "^6.0.49",
    "lodash": "^4.17.19",
    "moment": "^2.27",
    "@popperjs/core": "^2.11.8",
    "postcss": "^8.3.1",
    "pusher-js": "^4.4.0",
    "resolve-url-loader": "^5.0.0",
    "sass": "^1.63.4",
    "sass-loader": "^12.1.0",
    "sortablejs": "^1.4.2",
    "tailwindcss": "^1.9.6",
    "vue": "^2.7.14",
    "vue-loader": "^15.9.8",
    "vue-template-compiler": "^2.7.14"
  },
  "dependencies": {
    "mousetrap": "^1.6.5",
    "vuedraggable": "^2.24.1"
  }
}

And the prettier config is the default from the readme:

{
    "plugins": [
        "./node_modules/prettier-plugin-blade/"
    ],
    "overrides": [
        {
            "files": [
                "*.blade.php"
            ],
            "options": {
                "parser": "blade"
            }
        }
    ]
}
JohnathonKoster commented 1 year ago

The .prettierrc file looks okay, but I don't see prettier-plugin-blade within the package.json contents. Has the package been installed within the project, and available within node_modules?

npm install prettier-plugin-blade
joedixon commented 1 year ago

My bad - I had it installed when I ran into the formatting issues, but removed it temporarily while we discuss the issue.

JohnathonKoster commented 1 year ago

That's fair - would you be able to supply the full Blade template as an attachment (to help preserve the original whitespace/etc.)? I'm having a lot of difficulty reproducing the issue at this time.

joedixon commented 1 year ago

Don't think I need to as I've managed to find a minimal repro. Seems to be related to escaping tags which I guess means it's a gnarly one to solve?

<div>
    <h1>@{{ user.name }}</h1>

    <div>@if($user->avatar) @include('users.avatar')@endif</div>
</div>
JohnathonKoster commented 1 year ago

I appreciate taking the effort to find a minimal repro. Unfortunately, I still cannot reproduce the issue:

https://github.com/Stillat/blade-parser-typescript/assets/5232890/6dd6b48d-b9df-4f97-81f9-153ade676ccd

joedixon commented 1 year ago

There is no indentation of the the @if directive. It shows with the simple repro, but the issue is highlighted if I make the conditionals more complex as per my original post.

https://github.com/Stillat/blade-parser-typescript/assets/3438564/e1b25514-91df-4470-bc45-def426f464f4

https://github.com/Stillat/blade-parser-typescript/assets/3438564/91c38f19-9425-497e-a2bd-6f1dcafcd9c0

JohnathonKoster commented 1 year ago

Thanks for the video! I had added some whitespace while transcribing/testing earlier. Can reproduce now.

Apologize for all the back and forth!

joedixon commented 1 year ago

Gotcha, no problem. Lmk if there is anything I can do to help.

JohnathonKoster commented 1 year ago

The issue with the escaped content causing unexpected results has been addressed in 1.6.6. Thank you for all your patience and help!

joedixon commented 1 year ago

Amazing, thank you!

joedixon commented 1 year ago

Can confirm this is working great!