I'm looking into this today. So far I've found that none of the code in AddLinkArticle_target.js runs on page reload.

In T278485#6953982, @mepps wrote:

I'm looking into this today. So far I've found that none of the code in AddLinkArticle_target.js runs on page reload.

FWIW the inspector loads if you switch from mobile to desktop, so that probably can give some clue of how to fix on desktop.

@Tgr mentioned in T277228: VE plugin fails to load on desktop article _target that the current solution for loading add link plugin on desktop won't address the issue when the page is reloaded, so this issue might be fixed when the real fix for T277228 is in. I think we should keep this bug report separate for now though in case there are further issues that arise even when the plugin is loaded given that there are already a few issues w/link inspector upon initial load.

If I'm on the read page, and refresh, then click edit, I get AI suggestions. But it only doesn't work if I refresh on edit. Is there a way to use this? (Like @mewoph's earlier solution for T277228).

addPlugin (from mw.hook( 've.loadModules' )) doesn't do anything if loadModules in ve.init.mw.Article_targetLoader has already been called. In this case, ext.growthExperiments.SuggestedEdits.Guidance.js needs to be loaded and addPlugin needs to be called before loadModules is called.

I added some more logs and noticed that the sequences of events are different between the first load and the reload attempts.

First load — inspector shows up

Reload on edit — inspector doesn't show up

Reload on read & manually click edit — inspector shows up

The discrepancy in the sequence of events lines up with the different behaviors we are seeing here.

Given these constraints, for reload to work, I think we would either have to guarantee that guidance.js is loaded before VE or to not use addPlugin if loadModules has already been called by the time guidance.js loads. I'm not sure how to achieve the former. As for the latter, I tried checking for the presence of ve.init.platform and inspector showed up on reload.

In guidance.js:

			mw.hook( 've.loadModules' ).add( function ( addPlugin ) {
				// Either the desktop or the mobile module will be registered, but not both.
				// Start with both, filter out the unregistered one, and add the remaining one
				// as a VE plugin.
				[
					'ext.growthExperiments.AddLink.desktop',
					'ext.growthExperiments.AddLink.mobile'
				].filter( mw.loader.getState ).forEach( function ( module ) {
					if ( window.ve && ve.init && ve.init.platform ) {
						mw.loader.using( module );
					} else {
						addPlugin( module );
					}
				} );
			} );

@mepps @Tgr @kostajh — do you think this makes sense as a solution to this?

Is there a reason we need to use addPlugin if it works to load the modules without it?

Change 678012 had a related patch set uploaded (by Mepps; author: Mepps):

[mediawiki/extensions/GrowthExperiments@master] Use mw.loader.using instead of addPlugin to load AddLink

https://gerrit.wikimedia.org/r/678012

@mewoph I put up a patch that does not include the if statement because I found that worked for me locally. However, I understand there are nuances to the loading times, so if addPlugin is needed, feel free to update the patch I put up.

Thanks @mepps I'm not sure why addPlugin was used. Maybe @kostajh or @Tgr have more context?

From the documentation of mw.libs.ve.addPlugin (which is a different addPlugin method, but as far as I can tell it does the exact same thing):

Plugins are run after VisualEditor is loaded, but before it is initialized. This allows plugins to add classes and register them with the factories and registries.

So, if the plugin code runs too early, VE code is not loaded yet; if it runs too late, chances are the factories and registries have already been used to create the editor, and changing them will have no effect. Using addPlugin avoids running too early. (Ideally it should also warn / error out when trying to run too late - I think not doing that is a bug in the VE code.)

mw.loader.using is not a safe replacement IMO, it will suffer from the same problem. We need to make sure the code where we register for the ve.loadModules hook runs sufficiently early.

The documentation also says this:

No special care is taken to ensure that the module runs after the VE classes are loaded, so if this is desired, the module should depend on ext.visualEditor.core.

If [the argument of addPLugin is] a function, it will be invoked once the VisualEditor core modules and any plugin modules registered through this function have been loaded, but before the editor is intialized. The function can optionally return a jQuery.Promise. VisualEditor will only be initialized once all promises returned by plugin functions have been resolved.

Maybe that can be useful here.

No special care is taken to ensure that the module runs after the VE classes are loaded, so if this is desired, the module should depend on ext.visualEditor.core.

Thanks @Tgr! Wouldn't we need the opposite guarantee in order to use addPlugin (to make sure that ve.loadModules registration happens before VE is initialized so that when VE gets initialized our plugins are loaded in the correct order)?

Alternatively, assuming that adding ext.visualEditor.core as a dependency guarantees that the module will be run at the right time, would it make sense to add ext.visualEditor.core to ext.growthExperiments.AddLink.desktop's dependencies and load the module outside of ve.loadModules hook (not use addPlugin)?

Change 679797 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] AddLink: Enable plugin to load on page reload

https://gerrit.wikimedia.org/r/679797

In T278485#6996614, @mewoph wrote:

would it make sense to add ext.visualEditor.core to ext.growthExperiments.AddLink.desktop's dependencies and load the module outside of ve.loadModules hook (not use addPlugin)?

That's already the case -- we depend on ext.visualEditor.mwlink which depends on ext.visualEditor.mwcore, and that depends on ext.visualEditor.core

Change 679797 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] AddLink: Enable plugin to load on page reload

https://gerrit.wikimedia.org/r/679797

Change 678012 abandoned by Gergő Tisza:

[mediawiki/extensions/GrowthExperiments@master] Use mw.loader.using instead of addPlugin to load AddLink

Reason:

Was solved another way in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/ /679797

https://gerrit.wikimedia.org/r/678012