-
Notifications
You must be signed in to change notification settings - Fork 19
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
Remove final from render #105
Conversation
No finals!
Ditch phpstan ignores
allow variables to be preserved if manually added
@@ -79,9 +79,11 @@ protected function requirePath(string $path): void | |||
: $variable; | |||
}, get_defined_vars()); | |||
})(); | |||
CompileContext::instance()->variables = array_merge(CompileContext::instance()->variables, $variables); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this unrelated change made?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@taylorotwell sorry, yep this was needed further down the track of my implementation. The mingle lib needs a component
method defined to point to the react/vue component it should render, and I wanted to add it automatically as I'm extracting the component from the blade file itself (see https://github.com/deanmcpherson/mingle/blob/main/src/Mingle.php), but variables added to the default array before this is run were getting overridden here.
@@ -116,7 +116,7 @@ protected function registerTestingMacros(): void | |||
$component = FragmentMap::get($component); | |||
} | |||
|
|||
return $this->assertSeeLivewire($component); // @phpstan-ignore-line | |||
return $this->assertSeeLivewire($component); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this change made?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The static analysis in the github action for the PR threw an error because there was nothing to ignore anymore. I suspect the original suppression was because of the use of final? Happy to revert
Can we please remove final from the render method?
I'm playing with adding support for rendering react/vue components inside of volt components with https://github.com/ijpatricio/mingle, but naturally it needs to be able to hijack the render method to function.
I can't think of any reasons why removing the final keyword would be a breaking change here.