- Issue created by @mxr576
- 🇦🇹Austria drunken monkey Vienna, Austria
drunken monkey → made their first commit to this issue’s fork.
- 🇦🇹Austria drunken monkey Vienna, Austria
Thanks for suggesting this improvement! I agree that in theory it makes sense to add this parameter and pass the plugin definition to
supportsIndex()
.However, there is of course a large problem when adding a new parameter to an interface, namely backwards-compatibility. We’d have to add it in a commented-out way (see #3128548: Add optional parameters to StatementInterface::fetchObject() to be in line with the PDO implementation of the method fetchObject() → for an example from Core), adapt our implementations and hope that other classes implementing the method will also do that before we change the signature for real in a future 2.0.0 release of the module. (Based on a sentence in the IS of 📌 [11.x] Adjust parameters in interfaces Active , which discusses making that same Core change permanent, it seems like Symfony’s debug classloader will understand this syntax of commenting out parameters, so people might at least get notified.)
While it seems there is still a clear way to do this, it does carry the risk of breaking people’s code when we release version 2.0.0, and I’m not sure the use case you describe is common enough to warrant this effort and risk. I’m keeping this open for now in case others would be in favor of this change, too, but am currently sceptical about whether we should do this.
- 🇭🇺Hungary mxr576 Hungary
Thanks for the feedback!
Yes, I am also aware of the complexity of changing interface contracts; however as you mentioned, it is also a well known and standard procedure even outside of Drupal. Moreover, there is a recent interface change in Drupal core that I am aware of: https://www.drupal.org/node/3459274 →
PHPStan spots interface changes even on level 1 and PHPStan is native feature on Drupal CI and I am hopeful that nobody disables it ;S