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

Module shouldn't rely on magento2-base package to exist #45

Closed
hostep opened this issue Jun 18, 2021 · 3 comments
Closed

Module shouldn't rely on magento2-base package to exist #45

hostep opened this issue Jun 18, 2021 · 3 comments

Comments

@hostep
Copy link
Contributor

hostep commented Jun 18, 2021

Hi guys!

As a best practise, we delete the vendor/magento/magento2-base module when deploying a Magento shop to a server.
The code in this module is not used when running a Magento shop, its only purpose is to copy files inside it to the root of the project using a composer plugin. But when that is finished, it serves no longer a purpose anymore. So we remove this module always in order to save disk space.

However, it looks like your module relies on the composer.json file to exist in vendor/magento/magento2-base to figure out the current Magento version, this is done over here:

file_get_contents(BP . '/vendor/magento/magento2-base/composer.json'), $matches)) {

Is there a chance you could try to revert back to using $productMetadata->getVersion() as mentioned in the comments. It is slower indeed, but Magento seems to have fixed some slowness that was reported. This should be faster now in Magento 2.4.0 and higher by magento/magento2#24030 and magento/magento2#26001.

Would you consider trying to remove the reliance on the vendor/magento/magento2-base module from this module? Or maybe add a fallback for when that vendor/magento/magento2-base/composer.json file is not found to still use $productMetadata->getVersion()? That would be appreciated 🙂

Thanks!

@patrick-bigbridge
Copy link
Contributor

Hello @hostep

That's an interesting note you make there about magento2-base. I had not heard that before.

Since speed is essential to the module, I am not happy with anything that slows it down. But providing a fallback is perfectly acceptable, of course. I will add it somewhere in the next few weeks.

@patrick-bigbridge
Copy link
Contributor

Version 1.7.2 now contains the fallback.

@hostep
Copy link
Contributor Author

hostep commented Jun 21, 2021

Awesome, thanks for the quick fix!

@hostep hostep closed this as completed Jun 21, 2021
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