search

laminas / laminas-component-installer

Composer plugin for injecting modules and configuration providers into application configuration
https://docs.laminas.dev/laminas-component-installer/
BSD 3-Clause "New" or "Revised" License
27 stars 12 forks source link

Add ConfigProvider or Module when upgrading package #10

Closed boesing closed 3 years ago

boesing commented 4 years ago

Feature Request

Q A
New Feature yes

Summary

When upgrading a package which did not provide a ConfigProvider/Module in the initial version, the plugin does not add missing ConfigProvider/Module.

Thats actually the case for laminas-diactoros v2.2 to laminas-diactoros v2.3 which introduced the ConfigProvider to register PSR-17 interfaces.

I'd love to see ConfigProvider/Module is being injected when updating a package aswell as installing/uninstalling it.

boesing commented 4 years ago

This feature might probably prevented https://github.com/mezzio/mezzio-cors/issues/8

weierophinney commented 4 years ago

This is actually hugely problematic, and it's something we chose not to do after getting user complaints.

The problem is if we do it at update, then if the user chose not to install it during initial install (e.g., they want to autoload, but do not want services registered), it is now prompting them to inject it again during an update. And if they are not expecting that, and another package prompts prior to that and they answer "yes" when asked if they want to do the same for any other packages... they're now having it injected when they don't want to.

The only way we'd be able to manage this is to write to a file somewhere about the installation choices previously made, and that then means users have to commit that file in their repo, which leads to other problems as well.

geerteltink commented 4 years ago

What if we use composer.json to store that info?

boesing commented 4 years ago

What if we use composer.json to store that info?

yeah, was thinking about that when working on the dependency-plugin aswell. Storing "no" in composer.json might solve that problem.

But one could compare previous and new composer.json during update aswell and just ask if the previous Version did not had that "config-Provider" extra config. There is a build-in logic for that in the "UpdateOperation" object.

boesing commented 3 years ago

I've create a PoC in boesing/laminas-component-installer#2 It needs #25 to be merged before I can create a PR in here.

Reviews welcome.

boesing commented 3 years ago

Added #28 as #25 got merged today. 🎉

boesing commented 3 years ago

@weierophinney

The problem is if we do it at update, then if the user chose not to install it during initial install (e.g., they want to autoload, but do not want services registered), it is now prompting them to inject it again during an update. And if they are not expecting that, and another package prompts prior to that and they answer "yes" when asked if they want to do the same for any other packages... they're now having it injected when they don't want to.

The current implementation in #28 would prevent prompting over and over again. It would only prompt when removing vendor and re-installing (but thats the case right now aswell).

~I'll create a bug issue for that as thats something which annoys me aswell.~

~Due to laminas-validator and its suggest for laminas-db, I have to require laminas-db even tho we are not using it, so that psalm does not die with an error.~ ~As we do not need laminas-db at all, we do not add it to the modules.php but everytime I am cleaning my vendors directory to re-install deps, laminas-component-installer remembers me that we have to require laminas-db as dev-dependency for psalm.~

Looks like this got fixed in v4.7 of psalm. 🤷🏼‍♂️