Head is passing w D11
How is this possible though? What is the definition and table?
Are you sure something isnt malformed or altered elsewhere?
Issue template needs to be fixed.
Working on this...
I think the coverage at \Drupal\Tests\Core\Controller\TitleResolverTest
is sufficient.
Other than a tonne of implicit coverage.
PageVariantInterface
is just passing valid values down from TitleResolver::getTitle
Can we look into and potentially close 📌 Add a test for re-parenting of menu links. Postponed: needs info if there is sufficient overlap.
Created a autoloading-related issue for ensuring classes are added to the autoloader before container cache is unserialized @ 🐛 Using instances of classes defined by modules in service container parameters causes fatal errors Active
I did find
📌
Extension System, Part IV: Properly register all installed extensions/namespaces during container generation
Needs work
, which is more about comprehensive/unified namespaces for everything. E.g themes are added to autoloader in a crazy place (\Drupal\Core\Extension\ThemeHandler::addTheme
). Distinctly different to this issue.
Time has passed, new perspectives and such.
Following on a suggestion and discussion @ #3522410-11: Using instances of classes defined by modules in service container parameters causes fatal errors →
What if we did the inverse to what is currently proposed here, back to the original intent of the ticket before direction change in #2536610-6: Ensure all modules are autoloaded with PSR-4 only if enabled →
Always have all modules available in autoloader.
Then, the concept of "installed" means whether a modules hooks/services/discovery/plugins etc etc is loaded.
Converting what should be a Kernel test to a Functional test in order to get different PHP process is working out! 🎉 Still open to ideas if anyone else knows better.
Coverage
Critically, an error is shown when the new classLoaderAddMultiplePsr4
call in !12038 is removed:
Exception: Warning: unserialize(): Class 'Drupal\autoloader_test\AutoloaderTestEnum' not found
Drupal\Component\Serialization\PhpSerialize::decode()() (Line: 21)
When restored, the test pass once again.
NR
Thanks for the suggestion @godotislate, however I don't think thats works.
Problem is I need to add an enum or class reference to the container then get a new kernel either by a new PHP process or programmatically _rebooting_ it, so DrupalKernel uses the warm container cache. However since the test is all the same process, PHP already knows the class exists and caches it (AFAICT), so modifying the autoloader after the fact wont work.
Perhaps I will need a web test since I can rely on fresh PHP processes...
Pushed !12038
---
I've pushed up a PR which seems to fix the problem.
I'm at a loss for how to test this, as core/tests/bootstrap.php always adds all modules to the autoloader. I've tried a bunch of workarounds but I cant seem to get far. I'd appreciate if anyone can give that a go...
The flow of DrupalKernel has been moved around a bit, and the nearby type handling has been improved to satisfy PHPStan better, for even PHPStan levels core doesn't enforce yet. Especially with how core is passing around this semi-endorsed "container definition array" to various subsystems, for which I've specifically added a @phpstan-type
for.
The way the PR currently works is there is a new cache entry in cache.container
cache bin with a cache key with a different prefix to the entry with the entire container definition array.
A new ContainerDiscovery
class is implemented to hold onto discovery related data. Objects beat arbitrary arrays. And we can extend it later if we need other discovery related things added to it.
The flow of where classLoaderAddMultiplePsr4</code. is called has been changed up. It isnt evident to me why <code>classLoaderAddMultiplePsr4
is currently in attachSynthetic
, and it doesnt look like its relevant to that method as state and namespaces are not modified there. Perhaps it was more relevant many refactors ago. So it has been moved out of attachSynthetic
, up into initializeContainer
, so I can track and prevent doubling up of calls to classLoaderAddMultiplePsr4
Some parts of DrupalKernel have been changed to be more concise especially when a "discovery" edition of a code block is introduced alongside an existing "container definition" block.
fjgarlin → credited dpi → .
Merged this simple workaround.
I believe there is an issues somewhere..?
It would be nice to have code always-available, and simplify what "install" means. As in an "enabled" module (etc) adds it do discovery and namespaces, and which hooks are executed.
@astonvictor irt your activity on other issues, Ill take a look when I'm back from vacation. For now Im sighting things piecemeal.
This is necessary to be able to run linting (make lint) where auditfiles is the root project itself.
I'm not sure what issue youre reporting? What problem is this causing?
Notably I could tear apart the hook theme regeneration stuff in compiler pass and move it to a service, but Id still need to construct hook_theme default variables by looking thru definitions, then doing reflection stuff. Which is a PITA.
...or nullify all default theme variables
This issue might be suitable to also update \Drupal\Component\DependencyInjection\Container::getParameter
to allow enum returns.
Method signatures
\Symfony\Component\DependencyInjection\ContainerInterface::getParameter
public function getParameter(string $name): array|bool|string|int|float|\UnitEnum|null;
\Drupal\Component\DependencyInjection\Container::getParameter
public function getParameter($name): array|bool|string|int|float|NULL {
--
And somewhat related, looking to address use of enums in parameters causing unserialisation on warm loads blowing up since the autoloader isnt fully populated, at @ 🐛 Using instances of classes defined by modules in service container parameters causes fatal errors Active
The result is similarly catastrophic for _simpler_ data types stored in the container, like enums.
Enums are a valid return type of getParameter() in Symfony, but Drupal has not been updated to support them.
New minor as its a soft break to internal functionality.
Notably, our private "common" library uses it via setPintoHookTheme
, so will need 0.5.1, and updates to code:
- param type array -> string
- add unserialize before passing to array_keys
There shouldn't be any runtime performance penalty, as the result of \pinto_theme is subsequently cached elsewhere on first-use.
Any custom code using pinto.internal.hook_theme
must also follow our lead, by unserialising. 🤮
Critical points:
DrupalKernel
if (empty($this->moduleList) && !$this->containerNeedsRebuild) {
$container_definition = $this->getCachedContainerDefinition();
}
and further down, where module PSR4 namespaces are added:
$this->attachSynthetic($container);
dpi → created an issue.
Since theres no direct answer to #7, closing this so there is no wasted effort in this issue, compared to the long-lived/discussed/followed issue at ✨ Allow components to define asset libraries Needs work .
Please open if differences can be justified.
Coverage pushed.
dpi → created an issue. See original summary → .
Please do not spam tags.
To prevent notifications getting routed to catch-all channels, like Chat, this should by default error out (exception) if a notification type is not covered by a configured interface -> channel/transport mapping
This should help with scenarios like:
Notification is routing to a Slack recipient.
The mercure transport is enabled (is a chat channel transport)
The mercure transport will accept the notification with slack recipient by default.
We add something that can limit unintended transports.
\Drupal\notifier_chat_channel\Transport\Slack\Recipient\SlackRecipientInterface: chat/slack
IF a message is routed which dosent match a class/iface/abstract etc combo, then throw exception.
This should reduce the number of custom channelrouter services needed, and also help to prevent unintended message routing if for example more transports within a channel are added progressively.
The first implementation has been added, for Slack.
Looks like we'll need to do this for each provider, and adapt concepts from each.
Now that we know the required effort, we can do that in separate issues per service.
Incl notification and channel router, per README
dpi → created an issue. See original summary → .
With big codebases, we're doing php-driven autoregistration without services.yml (as core decoupled from sf and is still bikeshedding about that).
Quite some dev dudes i know with big code bases do it like this.
Thanks @geek-merlin, this makes sense, and allows me to approve this issue. I also prefer everything in `src` as a service, relying on private/service removal if unused, per Symfony.
Perhaps you'd be interested in taking a look at a core ticket I created a year ago: ✨ Directory based automatic service creation Needs work to help de-bikeshedding core ;)
Something needs to be done about the HuxCompilerPass2 name, and I'll do a deeper review. This will happen when I can get to it next, but isnt a high priority. Tests, docs, updates also
In the case where multiple chat channels are configured, greedy channels like Mercure (supports() has minimum requirements), ensure filters filter out for example where a message needs to go to Slack and never mercure.
The new CanonicalProduct and Slots attr via php extends inheritance should fulfill the requirements here.
With these systems, you can sub in your own twig/js/css, but inherit from slots and a parent object.
Futher, Slots are the recommended way of working, and they dont have a much surface thats hookable.
If these are not a good solution, let me know!
For #3, for now, changed to exception if missing file, unless the asset is a glob in which no exceptions are thrown if nothing was found.
Also finding that adding asset on a List, where not all components have an asset with that asset name, an error is shown.
As a part of this we should be able to validate whether a file truly exists, and add it conditionally.
Maybe consider adding a param to Asset attr that can ignore non existent files, or throw errors on missing files (default).
Of course, otherwise the Asset attr can be added to individual components/enum-case rather than an asset-attr thats echo'ed from the top of an enum.
We'll do it outside of BCA for now, but may revisit due to need for a flexible weighting system.
Theres been discussions of if or how to scan vendor for plugins before, I dont have references on hand.
I think core would want to look at composer autoload namespace and directories and scan them; whereas the change here is a simple list of which classes to include. No directory scans.
For selfish reasons I dont really have time on hand to look through core, as this is very much a side thing of what I'm working on for PNX. I'd prefer a quick merge but otherwise will advise to use a patch in the meantime.
container.namespaces includes only Drupal extensions.
The desire for this issue is to include classes defined in vendor.
Tests can be arranged if we can agree on implementation/direction.
Cred for auto-invoke idea.
e67ead44 includes integration for 0.2.0 except for Asset globs, which need more time as I found typical issues related to how Drupal libraries are relative to app root, and the generic local asset attributes, Css and Js, don't know how to find files (unless they are absolute).
Deferred and will be integrated independently, after probably more abstraction work in dpi/pinto lib @ ✨ Add support for Asset globs Active
For SMS Framework v4, the topics discussed here can be achieved with https://comms.docs.contrib.social/sms/verification/code-generator
For SMS Framework v4, the topics discussed here can be achieved with https://comms.docs.contrib.social/sms/verification/conditional
Project is obsolete now that SMS Framework provides support for services, including Clickatell.
Project is obsolete now that SMS Framework provides support for services, including Clickatell.
Project is obsolete now that SMS Framework provides support for services, including Clickatell.
Project is obsolete now that SMS Framework provides support for services, including Clickatell.
Project is obsolete now that SMS Framework provides support for services, including Clickatell.
Project is obsolete now that SMS Framework provides support for services, including Clickatell.