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

fix: resolve Blade component class namespace issue by removing redundant 'app' prefix in the service provider and configuration. This caused components to not be recognized in Blade when variables were passed in the component's constructor. #2010

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

reza-asgharzadeh
Copy link

@reza-asgharzadeh reza-asgharzadeh commented Jan 23, 2025

fix: resolve Blade component class namespace issue by removing redundant 'app' prefix in the service provider and configuration. This caused components to not be recognized in Blade when variables were passed in the component's constructor.

The issue was caused by the redundant 'app' prefix in the service provider and configuration, while the prefix is already defined in the PSR-4 configuration. As a result, the component class was not correctly linked to Blade, and variables defined in the component's constructor were not recognized. By removing the redundant 'app' prefix, the issue was resolved.

…pp' prefix in PSR-4 configuration. This caused components to not be recognized in Blade when variables were passed in the component's constructor.
@reza-asgharzadeh reza-asgharzadeh changed the title fix: resolve Blade component namespace issue by removing redundant 'app' prefix in PSR-4 configuration. This caused components to not be recognized in Blade when variables were passed in the component's constructor. fix: resolve Blade component class namespace issue by removing redundant 'app' prefix in the service provider and configuration. This caused components to not be recognized in Blade when variables were passed in the component's constructor. Jan 23, 2025
@solomon-ochepa
Copy link
Contributor

Nice work. Please keep the title short nice time. You can always add a description to explain what the PR does.

@@ -116,7 +116,7 @@
'channels' => ['path' => 'app/Broadcasting', 'generate' => false],
'class' => ['path' => 'app/Classes', 'generate' => false],
'command' => ['path' => 'app/Console', 'generate' => false],
'component-class' => ['path' => 'app/View/Components', 'generate' => false],
'component-class' => ['path' => 'View/Components', 'generate' => false],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are expected to change the value to whatever you wish in your published config file.

This default follows Laravel's standard structure and should remain unchanged.

@@ -109,7 +109,7 @@ class $CLASS$ extends ServiceProvider

$this->loadViewsFrom(array_merge($this->getPublishableViewPaths(), [$sourcePath]), $this->nameLower);

$componentNamespace = $this->module_namespace($this->name, $this->app_path(config('modules.paths.generator.component-class.path')));
$componentNamespace = $this->module_namespace($this->name, config('modules.paths.generator.component-class.path'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are expected to change the value to whatever you wish in your published config file.

The default follows Laravel's structure, but $this->app_path() does not enforce app/. You can leave it empty or set any value through the config.

@solomon-ochepa
Copy link
Contributor

This PR is a NO from me.

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

Successfully merging this pull request may close these issues.

2 participants