`artifactUrl()` should accept absolute URIs

[AugustMiller]

In the process of setting up our website for local dev, I ended up sending the Webpack dev server’s absolute URI to Helper::artifactUrl(). Our assetrev.php file

The Cloud extension treats all input like a path (represented by an instance of League/Uri/HierarchicalPath), flattening the scheme’s // into a single slash. In reality, it should probably permit and pass through absolute URIs, so that it retains parity with Craft's built-in siteUrl() and url() functions.

{# These work as expected: #}
{{ siteUrl('https://craftcom.ddev.site:8085/dist') }}
{{ url('https://craftcom.ddev.site:8085/dist') }}

{# This throws an exception: #}
{{ artifactUrl('https://craftcom.ddev.site:8085/dist') }}

In all three cases, the values actually come from an environment variable, or defaults to a path (dist).

[timkelty]

I'm not sure I agree that it should take a full URL…

it should probably permit and pass through absolute URIs

So, when served by Cloud, given {{ artifactUrl('https://craftcom.ddev.site:8085/dist') }}, are you saying that it should return https://craftcom.ddev.site:8085/dist or would it attempt to swap the origin and make it work as a real artifact url?

It seems quite different than siteUrl and url, in that those functions will just return the full url you provided it, where that would never really be what you wanted with artifactUrl.

An alternative would be to introduce a CRAFT_CLOUD_ARTIFACT_BASE_URL config setting to override the default @web base when local.

Curious what @kennethormandy thinks, as he's probably dealt with this the most.

[AugustMiller]

Maybe this is a little less black-and-white than I thought…

So, when served by Cloud, given {{ artifactUrl('https://craftcom.ddev.site:8085/dist') }}, are you saying that it should return https://craftcom.ddev.site:8085/dist or would it attempt to swap the origin and make it work as a real artifact url?

Originally, yes, that was what I imagined. If it receives an absolute URL as an argument, it would be passed through unchanged.

(Going further down this path, it would make sense to support the second argument, like url() and siteUrl(), to modify the final query params output with the URL…)

It seems quite different than siteUrl and url, in that those functions will just return the full url you provided it, where that would never really be what you wanted with artifactUrl.

Ok, I think I may just be assigning a broader use case to this than is expected; its actual intent is to work as a drop-in replacement for any uses of siteUrl() and url() that refer to a static file in your web root. What I was encountering was an environment-specific need to replace that URL with something that points to a fundamentally different HTTP server, with no relationship to our web root or site URL.

In other words, the usage context is not the only thing that determines what helper I should use; the expected output/values are actually different. I think the correct thing to use here is probably a ternary statement with Helper::isCraftCloud()?

# assetrev.php

return [
  'assetUrlPrefix' => craft\cloud\Helper::isCraftCloud() ? Helper::artifactUrl('dist') : craft\helpers\App::env('WEBPACK_DEV_SERVER_BASE_URL'),
];

…where .env has…

WEBPACK_DEV_SERVER_BASE_URL="https://craftcom.ddev.site:8085/"

[AugustMiller]

I've updated the build artifacts article with this clarification!

[timkelty]

I think the correct thing to use here is probably a ternary statement with Helper::isCraftCloud()

@AugustMiller as things are now, yes.

However, I still think we should provide a config to change the artifiactUrlBase, like I suggested, so you wouldn't need the ternary.

it would make sense to support the second argument, like url() and siteUrl(), to modify the final query params output with the URL

No argument with that.