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

Allow custom path as a base path #202

Merged
merged 3 commits into from
Mar 10, 2021
Merged

Allow custom path as a base path #202

merged 3 commits into from
Mar 10, 2021

Conversation

rodrigopedra
Copy link
Contributor

@rodrigopedra rodrigopedra commented Mar 8, 2021

When applying the custom directory structure as suggested by the "Laravel Beyond Crud" book, the event discovery fails as it uses the base path to guess the class names from the file names.

private function fullQualifiedClassNameFromFile(SplFileInfo $file): string
{
$class = trim(Str::replaceFirst($this->basePath, '', $file->getRealPath()), DIRECTORY_SEPARATOR);
$class = str_replace(
[DIRECTORY_SEPARATOR, 'App\\'],
['\\', app()->getNamespace()],
ucfirst(Str::replaceLast('.php', '', $class))
);
return $this->rootNamespace.$class;
}

So a Projector placed at ./src/App/Projectors/UsersProjector.php is guessed as this class Src\App\Projectors\UsersProjector, which does not exist.

By allowing a user to specify a custom config key one could add this config key:

'auto_discover_base_path' => base_path('src'),

And have the src part ignored.

As it is seems to be an edge case, I didn't add a config key on the config stub.

If you prefer me to do so, please let know and I will update the PR.

More reasoning here: https://github.com/spatie/laravel-beyond-crud-demo/issues/7#issuecomment-792965120 (private repo)

EDIT I pushed more commits to allow for a more descriptive config key

@rodrigopedra
Copy link
Contributor Author

Just for the record, I sent PR laravel/framework#36515 to core for a similar approach and it got merged today.

@brendt
Copy link
Collaborator

brendt commented Mar 10, 2021

Nice, thanks!

@brendt brendt merged commit c261c9b into spatie:master Mar 10, 2021
brendt added a commit that referenced this pull request Mar 10, 2021
Allow custom path as a base path
@brendt
Copy link
Collaborator

brendt commented Mar 10, 2021

@rodrigopedra
Copy link
Contributor Author

Thanks!

@rodrigopedra rodrigopedra deleted the patch-1 branch March 10, 2021 12:43
@rodrigopedra
Copy link
Contributor Author

@brendt just for completeness:

I woke up and was about to add a commit adding a default config key to the config stubs. Let me know if you think that is a nice to have.

@brendt
Copy link
Collaborator

brendt commented Mar 10, 2021

Feel free to send another PR :) It's not that big a deal since there is a default value, but it's indeed good to have it.

@rodrigopedra
Copy link
Contributor Author

Thanks, just sent PR #203

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