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

oEmebed resources not loaded in Craft Cloud environment #158

Open
svale opened this issue Aug 8, 2024 · 19 comments
Open

oEmebed resources not loaded in Craft Cloud environment #158

svale opened this issue Aug 8, 2024 · 19 comments

Comments

@svale
Copy link

svale commented Aug 8, 2024

Hi,

I'm not sure if this is an issue that should be logged here or with Craft Cloud.

It seems the plugin asset resources (CSS and JS) are not loaded when our CP is deployed to Craft Cloud environments

image image

The plugin works as expected in the local DDEV env.

Craft Pro 5.3.0.3

Plugins
Amazon S3 2.2.1
Asset Usage 4.0.0
CKEditor 4.1.0
Embedded Assets 5.1.1
Expanded Singles 3.0.0
Feed Me 6.2.1
Field Manager 4.0.2
Formie 3.0.0
Hyper 2.0.3
Imager X 5.0.1
Imager X Craft Cloud Transformer 1.0.0-beta.1
Many to Many 4.0.0
oEmbed 3.0.9
Retcon 3.2.0
SEO 5.1.1
Sprig 3.2.1
Vite 5.0.0

@reganlawton
Copy link
Member

@svale Hmm sorry to hear that you're having issues, I personally haven't used Craft Cloud before and will have to spin up a trial account. I'm booked time wise for Friday (Leaving work early for a festival) and Saturday (My birthday). I might be able to slot some time in on Sunday.

Now I know what a pain this is to deal with personally when you have to wait, so I'd suggest asking Craft Cloud if they have any additional information about ways it could be having issues. It could be a configuration issue rather than an actual problem with the plugin itself. That being said, if it's not, I'll differently be happy to help out when I'm available again I just don't want to slow you guys down too much.

FYI I'm based in Melbourne AEST to help out with timeframes

@svale
Copy link
Author

svale commented Aug 9, 2024

No worries, enjoy the festival and your birthday 🎉 !

@timkelty
Copy link

@reganlawton Please reach out if you need any help. Here or @timkelty on Discord.

@reganlawton
Copy link
Member

Hi @timkelty

Seems that the Craft Cloud has assets’ upgrades.

Can you confirm this: https://craftcms.com/knowledge-base/cloud-plugin-development

I was playing around yesterday and found that URL I'm just trying to get as much done before trying out on the trial version on Craft Cloud. Being open-source, I don't really want to risk having the cost hit my personal card 😅 want to help the community seeing as this plugin is used so much I want to have Craft Cloud support.

@reganlawton
Copy link
Member

@svale Ok with the help of the CraftCMS team and a little bit of troubleshooting I have a sandbox account and working site and can see the issue. I'll work on the fixes for you you, and others, tonight.

Hoping it shouldn't be a hard fix though.

@svale
Copy link
Author

svale commented Aug 12, 2024

That is super - thanks, @reganlawton!

@reganlawton
Copy link
Member

It's a new service so always bugs. Overall good system I'll just to find the cause and will be reviewing tonight

@reganlawton
Copy link
Member

@svale This will be the issue right here

image

I've made it some the system only gets the LINK and SCRIPT on the CP dynamically cos of an old issue.

I'll have to reengineer the asset handling and then it should be simple as that. Might be abit of trial and error so Craft Cloud is new but shouldn't be a crazy issue.

@reganlawton
Copy link
Member

OK! It seems the issue is with the handling of the registration of the build process of the CDN, as expected.

Current code:

Event::on(
    View::class,
    View::EVENT_END_PAGE,
    function(Event $event) {
        if (Craft::$app->getRequest()->getIsCpRequest() && preg_match('/^\/.+\/entries\//', Craft::$app->getRequest()->getUrl())) {
            $url = Craft::$app->assetManager->getPublishedUrl('@wrav/oembed/assetbundles/oembed/dist/js/oembed.js', true);

            echo "<script src='$url'></script>";

            $url = Craft::$app->assetManager->getPublishedUrl('@wrav/oembed/assetbundles/oembed/dist/css/oembed.css', true);
            echo "<link rel='stylesheet' href='$url'>";
        }
    }
);

Test code:

Craft::$app->getView()->registerAssetBundle(OembedAsset::class);

Event::on(
    View::class,
    View::EVENT_END_PAGE,
    function(Event $event) {
        if (Craft::$app->getRequest()->getIsCpRequest() && preg_match('/^\/.+\/entries\//', Craft::$app->getRequest()->getUrl())) {
            $url = Craft::$app->assetManager->getPublishedUrl('@wrav/oembed/assetbundles/oembed/dist/js/oembed.js', true);

            echo "<script src='$url'></script>";

            $url = Craft::$app->assetManager->getPublishedUrl('@wrav/oembed/assetbundles/oembed/dist/css/oembed.css', true);
            echo "<link rel='stylesheet' href='$url'>";
        }
    }
);

So basically I just added the extra code to check if it was just not being published cos the code we only want to show in the CP pages. This was done due to someone reporting YEARS AGO the assets were showing on the site and conflicting with admin panels.

I think the best way of dealing with this now that CraftCMS is much more advanced is just:

if (!Craft::$app->getRequest()->getIsSiteRequest()) {
    Craft::$app->getView()->registerAssetBundle(OembedAsset::class);
}

This will have the CP working as expected, the CLI level with pick up the asset bundle. @svale Can you try using my feature branch I'm messing around with to see if it works for your side aswell?

compose.json:

"wrav/oembed": "dev-feature/craft-cloud-support",

@svale
Copy link
Author

svale commented Aug 12, 2024

@reganlawton ,

this looks great! I've tested the feature branch locally (DDEV) and on Craft Cloud and it seems to work perfectly, both the backend and the field data in the twig frontend 🙌

image

@reganlawton
Copy link
Member

I'll work to update a release version today and will check my other plugins 🙌

@timkelty
Copy link

timkelty commented Aug 13, 2024

@reganlawton glad you got it working! Couple notes:

  • Since you are registering the asset bundle in your plugin's init method, you should sure make to only do it for web requests (not console ones), so something like:
        if (Craft::$app->getRequest()->getIsCpRequest() && !Craft::$app->getRequest()->getIsConsoleRequest()) {
            Craft::$app->getView()->registerAssetBundle(OembedAsset::class);
        }
  • Registering an asset bundle in the init method like this probably isn't a great idea to begin with, as it means those assets are going to be loaded on every CP requests, regardless of if they're needed. Assuming they're only needed when the field is rendered, it is probably more appropriate to move it to \wrav\oembed\fields\OembedField::getInputHtml.

@reganlawton
Copy link
Member

@timkelty Oh I was understanding that it was the console requests that we're generating the CDN assets.

This is great to know! Thank you, new system to understand 😅

@timkelty
Copy link

timkelty commented Aug 13, 2024

@reganlawton, a console request does generate the assets (craft cloud/asset-bundles/publish), but that is solely based on the presence of your wrav\oembed\assetbundles\oembed\OembedAsset class.

So, your assets were actually getting generated all along, but just weren't being linked to properly. Using registerAssetBundle is the normal/expected way of doing this, which is why it started working when you swapped out the custom code.

@reganlawton
Copy link
Member

Thank you for the clarity much appreciated 🙏 @timkelty

@svale
Copy link
Author

svale commented Aug 18, 2024

Just to add that excluding console requests when calling registerAssetBundle (i.e. !Craft::$app->getRequest()->getIsConsoleRequest()), or moving it form init, is essential. If not the plugin code will break the craft cli

image

@timkelty
Copy link

timkelty commented Aug 19, 2024

Just to add that excluding console requests when calling registerAssetBundle (i.e. !Craft::$app->getRequest()->getIsConsoleRequest()), or moving it form init, is essential. If not the plugin code will break the craft cli

image

FWIW, Craft::$app->getRequest()->getIsCpRequest() will always return false for a CP request: https://github.com/craftcms/cms/blob/5.x/src/console/Request.php#L55

@jeffreyzant
Copy link
Contributor

Is there a reason why the console request logic isn't implemented yet? I see a new release is created about half an hour ago, but that version (3.1.0) still breaks all ./craft cli commands for me.

@reganlawton
Copy link
Member

This has been updated and a new release created.

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

No branches or pull requests

4 participants