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

[ENH]: Javascript resources from reusable module are not included #4038

Open
mdmontesinos opened this issue Mar 21, 2024 · 9 comments
Open

Comments

@mdmontesinos
Copy link
Contributor

Possible bug as per described in #4037.

Modules that can be reused in others, like RichTextEdit, should be able to include its own Javascript resources (not css) but they are not loaded nor inserted in the body/head when the modules are rendered in Static mode.

As a workaround, all resources from a child component must be defined in the calling (parent) module.

@mdmontesinos
Copy link
Contributor Author

mdmontesinos commented Mar 22, 2024

I've been testing with "regular" modules that declare a ModuleInfo and their js resources are also not loaded.

They are correctly added in the body but not requested and therefore are not available. Only when adding the Reload property to the resource it gets loaded. As far as I'm concerned, they should be at least requested once on the first page load and if some initialization needs to be done later, the reload property must be set to true. Am I wrong?

EDIT: This only occurs if you start in a page and then navigate to the page that includes these modules. If you reload the page, the js resources are requested. Does this mean that all modules in static mode must declare their js with Reload=True?

Here's the body:
image

Only the NoEditable module has the reload property (and also the external script).

The network fetch:
image

And the available sources:
image

As you can see, js resources without Reload=true are not requested.

Also, Render mode is obviously Static.

@mdmontesinos
Copy link
Contributor Author

Something else to note is that the onUpdate function of a js resource set with Reload=True is called 8 times in a single page transition... (when returning back to the page where the module is)

image

The module in only included once in the page, with a single instance of it.

This could become problematic when many modules are reloaded and their onUpdate functions are more complex, perhaps adding too much overhead.

@sbwalker
Copy link
Member

sbwalker commented Mar 22, 2024

@mdmontesinos declaring the Resources in a module or theme tells the framework that they need to be included in the page content - it does not tell the framework that they need to be executed.

In particular, JavaScript is challenging because Blazor does not support it natively. When running in Interactive render mode, you need to create a JS Interop class to call JavaScript within the OnAfterRenderAsync method and the JavaScript needs to be provided in a very specific format. In Static render mode, Blazor has better support for JavaScript however due to the fact the Enhanced Navigation is enabled by default, it causes problems with the standard way of executing JavaScript on page load. The recommended way to get this to work is to use a technique provided by Mackinnon Buck of the Blazor product team - which essentially is what the new Reload property on the Resource class is leveraging. You can read more about it here:

https://github.com/MackinnonBuck/blazor-page-script

You can also read more about how it works here:

MackinnonBuck/blazor-page-script#9

So it may be necessary to modify your JavaScript to remove onload references. And it is necessary for the the onUpdate method to be called multiple times - because the content on the page which it is referencing may not have been rendered yet. So if you only want your module to respond to the event once, you will have to create your own logic within your component.

@mdmontesinos
Copy link
Contributor Author

So we do need to set Reload to true for every js resource in modules that will be used in static mode, right?

Also, I'm not very js proficient, but the current js generated in the module template seems to not be compatible with this approach.

/* Module Script */
var [Owner] = [Owner] || {};

[Owner].[Module] = {
};

Should be replaced with something like:

/* Module Script */
function initialize[Module]() {
    "use strict";

    const hello[Module]Function = () => {
        console.log('Hello World from [Module]');
    }
}

export function onUpdate() {
    initialize[Module]();
}

And I should just invoke it like hello[Module]Function() ?

@sbwalker
Copy link
Member

sbwalker commented Mar 22, 2024

@mdmontesinos if you take a look at the Arsha Theme (https://github.com/oqtane/Oqtane.Theme.Arsha/blob/main/Client/ThemeInfo.cs) I published this week you will notice that there are a variety of JavaScript resources - however the majority of them are JavaScript libraries which are not executable - they simply need to be included in the page output. There is only 1 JavaScript file which is actually executable and needs the Reload set to true.

@sbwalker
Copy link
Member

sbwalker commented Mar 22, 2024

The module.js file in the module template contains no implementation - it just demonstrates how to include the resource. An Interop.cs is also included in the Client - which is the way that JavaScript needs to be called in Interactive Blazor - but it has no example implementation either.

In general, JavaScript is the most difficult technology to integrate with Blazor... and Static Blazor does not make it much easier (due to Enahnced Navigation). I believe the general recommendation of avoiding JavaScript and using native C# in the UI is still 100% true for Static Blazor - however there are some cases where JavaScript cannot be avoided.

@mdmontesinos
Copy link
Contributor Author

Ok @sbwalker, thanks for the pointers!

@sbwalker
Copy link
Member

@mdmontesinos is Reload the right name for this property? It's accurate from a Blazor perspective but from a standard JavaScript perspective, Load might be more appropriate? I can envision that in Interactive Blazor there may be support for this property ie, it could call a Load method without having to write the JS Interop logic yourself in your module.

@mdmontesinos
Copy link
Contributor Author

@sbwalker I believe Reload is the right name for the property (compared to Load), at least when considering its use case in static mode. If you rename it to Load, I believe it could be confusing because, why wouldn't you set Load=True for all js resources? Don't I want to load them in my module? Reload kind of indicates better that the resource could be "unloaded" in some cases and you must use this property. Also, Reload relies on the use of the onUpdate which is triggered often, so it's important to stand that out in the naming. Lastly, and I might be wrong, but many Blazor/Oqtane developers are usually not very proficient in js as they come from typical .NET apps, so maintaining a Blazor perspective for the naming seems most appropriate for me.

@sbwalker sbwalker changed the title Bug: Javascript resources from reusable module are not included [ENH}: Javascript resources from reusable module are not included Apr 8, 2024
@sbwalker sbwalker changed the title [ENH}: Javascript resources from reusable module are not included [ENH]: Javascript resources from reusable module are not included Apr 8, 2024
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

2 participants