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

move blade directives after resolving #25

Merged
merged 1 commit into from
Sep 29, 2017
Merged

move blade directives after resolving #25

merged 1 commit into from
Sep 29, 2017

Conversation

bmichotte
Copy link
Contributor

By using Blade::directive from your register method seems to force blade to fire its resolving, causing other packages which load their directives afterResolving to fail.

See spatie/laravel-permission#458, albertcht/invisible-recaptcha#41 and related issues

Signed-off-by: Benjamin Michotte <bmichotte@gmail.com>
@drbyte
Copy link
Contributor

drbyte commented Sep 15, 2017

👍
While I haven't encountered this clash when using both laravel-impersonate and laravel-permission, I've seen invisible-recaptcha cause troubles for the reason explained here.

Definitely appears to be safest to wrap all Blade directive assignments into the afterResolving of blade.compiler as this PR proposes.

@MarceauKa MarceauKa merged commit 938d510 into 404labfr:master Sep 29, 2017
@MarceauKa
Copy link
Member

Thank you!

@Treggats
Copy link

@MarceauKa can you create a release for this?

I've had laravel-permission blade directives show up and by updating this package to "dev-master" they were fixed.

@bmichotte
Copy link
Contributor Author

@MarceauKa any chance you'd tag a new release soon or at least some eta ?

@bmichotte
Copy link
Contributor Author

As a workaround while we are all waiting for release, you can use this in your composer.json

"require": {
      "lab404/laravel-impersonate": "dev-master",
},
"config": {
    "preferred-install": {
      "lab404/laravel-impersonate": "source",
      "*": "dist"
    }
  }

@drbyte
Copy link
Contributor

drbyte commented Jan 19, 2018

v1.2.2 has now been tagged, and includes this fix.

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

Successfully merging this pull request may close these issues.

5 participants