Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
After upgrading from rc8 to rc9 I noticed some issues introduced by the MigrationHelper::alterPlugins method. There are already a few other issues about it but none that raise this specific problem. The problem is that it is instantiating all source plugins in order to check if they are of a particular type.
See https://git.drupalcode.org/project/inline_entity_form/-/blob/8.x-1.0-rc9...
Our source plugins build a list of urls in the constructor to be used in migrations. That can be a pretty expensive operation and now it's being called twice every time the migration is run. The first time it's called by this module when it's instantiated in the hook, and then as normal during the actual migration.
Steps to reproduce
Install rc9.
Proposed resolution
Rather than instantiating all source plugins first and then checking their type, we could utilise the getDefinition method of the plugin manager to check if the plugin definition contains one of the classes we are interested in.
Remaining tasks
Write a patch.
User interface changes
N/A
API changes
N/A
Data model changes
N/A
Issue fork inline_entity_form-3226253
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
achapComment #3
achapComment #4
geek-merlinSounds like a reasonable thing to do, improving performance and preventing break on sites with large number of migration plugins.
Comment #6
achapComment #7
joachim commentedThis doesn't just improve performance, it prevents crashes. If some migrations are defined programmatically, they don't necessarily have the necessary components and shouldn't be instantiated.
MR works for me! Thanks!
Comment #10
geek-merlinSo we have MR33 RTBC'ed.
@jonathan_hunt As you created MR46, you should explain what you did and why.
Comment #11
james.williams#3226253-8: hook_migration_plugins_alter is instantiating all source plugins does explain the addition in !46, though I suspect it doesn't need to be conflated with this issue. There's already #3208818: Migration Error: Cannot use string offset as an array in MigrationHelper.php on line 53 that looks to be addressing the same thing. So I could be wrong, but I think that means we just stick with !33 and let #3208818: Migration Error: Cannot use string offset as an array in MigrationHelper.php on line 53 address the issue @jonathan_hunt was trying to address?
Comment #13
geek-merlin@james.williams Thx, now i get it.
Yes, let's keep the changes atomic.
Thank ya all!
Ups, strange: "!33 can not be merged"
Comment #15
geek-merlinOK, now merged.
Thank ya all!
Comment #16
jonathan_hunt commented@geek-merlin @james.williams Sorry, I identified this issue as "in-flight" but I didn't see #3208818: Migration Error: Cannot use string offset as an array in MigrationHelper.php on line 53; that is what I was aiming to fix. Thanks for your work.
Comment #17
geek-merlin