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.

Sign up for GitHub

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

Jump to bottom

[10.x] Vite custom cdn support #49378

Closed
wants to merge 3 commits into from
Closed

[10.x] Vite custom cdn support #49378

wants to merge 3 commits into from

Conversation

AmirHossein
Copy link

assetsUrl added to be able to pass custom cdn.

@vite('resources/sass/app.scss', assetsUrl: 'https://cdn.domain/assets')

to have output like this:

<link rel="preload" as="style" href="https://cdn.domain/assets/app.c3369b22.css" />
<link rel="stylesheet" href="https://cdn.domain/assets/app.c3369b22.css" />

@AmirHossein
assetsUrl property added
28e243d 
assetsUrl added to be able to pass custom cdn
`@vite('resources/sass/app.scss', assetsUrl: 'https://cdn.domain/')`
@AmirHossein
cleanup
231d69d
@AmirHossein
InteractsWithContainer updated
bc517bd
@taylorotwell taylorotwell marked this pull request as draft December 14, 2023 15:16
@AmirHossein AmirHossein changed the title Vite custom cdn support [10.x] Vite custom cdn support Dec 14, 2023
@timacdonald
Copy link
Member

@AmirHossein can you not set the ASSET_URL environment variable? That is what the variable does already and it's intention.

@AmirHossein
Copy link
Author

in my case, asset url is not static and resources are distributed to different servers. so it would be great to support such cases...

@timacdonald timacdonald self-assigned this Dec 15, 2023
@ChrisB-TL
Copy link

This would be handy for multi tenant apps where the asset URL is determined dynamically.

@timacdonald
Copy link
Member

Would you just do the following in your tenant "setup" to achieve this?

Config::set('app.asset_url', '...some dynamic URL here based on the tenant...');

The app.asset_url config is not just used by Vite, so you would want that set property for all assets, right?

@ChrisB-TL
Copy link

ChrisB-TL commented Dec 18, 2023 •
edited
Loading

Would you just do the following in your tenant "setup" to achieve this?

Config::set('app.asset_url', '...some dynamic URL here based on the tenant...');

The app.asset_url config is not just used by Vite, so you would want that set property for all assets, right?

Ah, sorry, left out a key bit of info. We do the above for tenant subdomain assets. However, we serve bundles from our root domain.

We currently do this by registering a modified Illuminate\Foundation\Vite with assetPath() overridden - so this would just be a nice to have from an upgradability perspective rather than necessary for our use case. And I cannot imagine it would be affected by upgrades much if it at all...!

@timacdonald
Copy link
Member

timacdonald commented Dec 18, 2023 •
edited
Loading

Hey folks,

Would something like the following work for your needs? Similar to how we currently handle advanced customisations: https://laravel.com/docs/10.x/vite#advanced-customization

<head>
    <!-- ... -->
    {{
        Vite::withEntryPoints(['resources/js/app.js'])
            ->createAssetUrlsUsing(function ($path) {
                // Custom logic here.
            })
    }}
</head>

I'm not sure the current proposed solution is how we want to handle this.

But could one of you also provide a bit more detail on why the feature is needed with the other things I've mentioned.

Still not quite sure I get the setup that actually requires this change. Thanks!

@ChrisB-TL
Copy link

ChrisB-TL commented Dec 19, 2023 •
edited
Loading

That would work for our use case.

For us, any assets unique to a tenant are served from the tenant's domain using the asset_url config. Vite bundles are on the root domain only (and are the only thing on the root domain), which we use a CDN for. So we only override the Vite asset path.

Amir's use case sounds different, though.

FWIW in case you choose not to implement this and anyone is looking for an alternative, we currently just do this (The global_asset function is just a custom helper, that builds the URL in a similar way to asset):

use Illuminate\Foundation\Vite as BaseVite;

class MultiTenantVite extends BaseVite
{
    /**
     * Always serve assets from the main domain.
     */
    protected function assetPath($path, $secure = null)
    {
        return global_asset($path);
    }
}

and then in the AppServiceProvider:

$this->app->singleton(\Illuminate\Foundation\Vite::class, MultiTenantVite::class);

@AmirHossein
Copy link
Author

AmirHossein commented Dec 19, 2023 •
edited
Loading

Hi,
In my case, assets are managed in different level and provided via function in apps. So it is not static value.

We currently override assetPath method like @ChrisB-TL but asked for support officially, if possible.

As $path passed to the function i think createAssetUrlsUsing is enough.
Somethinbg like below:

Vite::createAssetUrlsUsing(function ($path) {
    // Custom logic here.
})

@timacdonald timacdonald mentioned this pull request Dec 19, 2023
Merged
@timacdonald
Copy link
Member

Hey folks, I've created #49437

Could you confirm this solves your problems. Note that it does not allow you to customize the "hot" path generation, i.e., when the dev server is running, only the built asset paths.

Let me know your thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timacdonald timacdonald Awaiting requested review from timacdonald

Assignees

@timacdonald timacdonald

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AmirHossein @timacdonald @ChrisB-TL