search

Kyon147 / laravel-shopify

A full-featured Laravel package for aiding in Shopify App development
MIT License
353 stars 102 forks source link

Include "host" in @sessionToken output #194

Closed michaellehmkuhl closed 12 months ago

michaellehmkuhl commented 1 year ago

Since the URL::tokenRoute() helper now includes the host parameter, it seems useful to include the same in the @sessionToken blade directive output for forms.

Kyon147 commented 12 months ago

@michaellehmkuhl could you just take a look at this for PHP 8.0 failing?

michaellehmkuhl commented 12 months ago

@Kyon147 it looks like it's the phpstan static code analysis step that's failing. That check seems to be disabled in the other package tests. It's only failing in this case because phpstan isn't able to resolve the Request class through Laravel magic. If you're OK with just adjusting your PHP 8.0 / Laravel 9.39.0 test to match the other tests (by disabling the phpstan check), then I think you'll see it passing.

michaellehmkuhl commented 12 months ago

@Kyon147 it crossed my mind that we could also probably get over the phpstan hurdle by making the class path explicit with a use Illuminate\Support\Facades\Request; so I went ahead and updated the code to reflect that approach.

Unfortunately, it doesn't look like that approach made phpstan happy, either.

michaellehmkuhl commented 12 months ago

@Kyon147 OK. Switched to the request() helper instead, and phpstan seems ok with that approach. All checks have passed! ✅

Kyon147 commented 12 months ago

@michaellehmkuhl thanks for taking the time to look at this and get the CI/CD happy. I've merged this in now, so will look at pushing out a patch release this week as it should be helpful to people using Blade.