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

Update override-existing-javascript.md #1556

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Nico-Schaefer-2111
Copy link

  • add note for overriding async JS-Plugin

- add note for overriding async JS-Plugin
@JoshuaBehrens
Copy link
Contributor

@aragon999

@aragon999
Copy link
Contributor

Thank you, but I think this is not an ideal solution, as the path depends on how the plugin is installed, and it is not always possible to know how a plugin is installed, for example when one has a store plugin, which extends another store plugin.

I tried to configure the one plugin as webpack path, to import it not via the relative path, but I did not manage to get it working. I will have a closer look into that when I am back from my holidays.

@Nico-Schaefer-2111
Copy link
Author

@aragon999 thanks, this kind of implementation is triggering me either. But I couldn't find another solution for this.

@Isengo1989 Isengo1989 added the Storefront ct-storefront label Nov 29, 2024
@tobiasberge
Copy link
Contributor

Hi, sorry for my late reply. I agree that this solution is not looking nice with the ugly path.

I think the problem is, that the prior example with getPlugin('PluginName') and then get('class') will not give you the class itself if it is async, because it was not downloaded yet.

Because the PluginManager.override() with the override-plugin containing the get('class') is happening BEFORE initializePlugins() which is downloading the plugins in the first place. For core plugins this is not a problem because everyone is likely just using the alias src/ to directly import it statically.

A webpack alias could be an alternative to avoid the path at least in the code. But like @aragon999 already said this can be tricky to determine the actual path. And it also requires a custom webpack config which was not needed before. We could try to automatically generate an alias for each bundle inside the core webpack config so that you have always something like @MyExtension1/something, @AnotherExtension2/something available.

Or solving it on a programmatic level, that we have something like fetchPlugin('MyAsyncPlugin') to force download a plugin before the actual initializePlugins so you are able to get the class for overriding it. But not sure honestly about the side-effects and drawbacks right of the bat without further testing.

@Isengo1989
Copy link
Collaborator

Hi, sorry for my late reply. I agree that this solution is not looking nice with the ugly path.

I think the problem is, that the prior example with getPlugin('PluginName') and then get('class') will not give you the class itself if it is async, because it was not downloaded yet.

Because the PluginManager.override() with the override-plugin containing the get('class') is happening BEFORE initializePlugins() which is downloading the plugins in the first place. For core plugins this is not a problem because everyone is likely just using the alias src/ to directly import it statically.

A webpack alias could be an alternative to avoid the path at least in the code. But like @aragon999 already said this can be tricky to determine the actual path. And it also requires a custom webpack config which was not needed before. We could try to automatically generate an alias for each bundle inside the core webpack config so that you have always something like @MyExtension1/something, @AnotherExtension2/something available.

Or solving it on a programmatic level, that we have something like fetchPlugin('MyAsyncPlugin') to force download a plugin before the actual initializePlugins so you are able to get the class for overriding it. But not sure honestly about the side-effects and drawbacks right of the bat without further testing.

Maybe it is better suited here then? -> https://github.com/shopware/shopware/discussions

As for helping developers till then, what about a more generic info, like: "For async plugins you need to import it via an absolute path where your plugin is located" (since it could also be located in custom/plugins etc.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Storefront ct-storefront
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants