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

Investigate Laravels asset helper #101

Open
ollieread opened this issue Feb 5, 2025 · 2 comments
Open

Investigate Laravels asset helper #101

ollieread opened this issue Feb 5, 2025 · 2 comments
Assignees
Labels
status: investigating The issue is being investigated status: planning The issue is being planned for the future type: fix Fix for a bug or other issue

Comments

@ollieread
Copy link
Member

Laravel's asset() helper makes use of APP_URL, which could be an issue when using something like subdomain routing.

A similar issue is mentioned in Trevors blog here: https://www.trovster.com/blog/2023/08/filament-panel-domain-issue

@ollieread ollieread added status: investigating The issue is being investigated status: planning The issue is being planned for the future type: fix Fix for a bug or other issue labels Feb 5, 2025
@ollieread ollieread added this to the V1 Release milestone Feb 5, 2025
@ollieread ollieread self-assigned this Feb 5, 2025
@ollieread
Copy link
Member Author

ollieread commented Feb 8, 2025

The asset() helper makes use of UrlGenerator::asset(), which generates the URL using several possible values for the host. Here they are, in order of precedence.

  • assetRoot - Set in the UrlGenerator constructor using the config value app.asset_url
  • cachedRoot - Set the first time UrlGenerator::formatRoot() is called
  • forcedRoot - Set by calling UrlGenerator::forceRootUrl()
  • Request::root() - Comes from the request

From what I can tell, in a standard Laravel and Filament install, UrlGenerator::forceRootUrl() is not called anywhere. It may be worth setting a root URL when a tenant is identified.

I don't think anything needs to be done with this, but I've created a PR to introduce a setAssetRoot method to help if it does need setting.

@ollieread
Copy link
Member Author

The PR laravel/framework#54530 was merged with a change of the name to setAssetOrigin

@ollieread ollieread moved this from Ready to Backlog in Sprout Development Roadmap Feb 12, 2025
@ollieread ollieread removed this from the V1 Release milestone Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: investigating The issue is being investigated status: planning The issue is being planned for the future type: fix Fix for a bug or other issue
Projects
Status: Backlog
Development

No branches or pull requests

1 participant