Page MenuHomePhabricator

Make MachineVision work with the Vue 3 migration build
Closed, ResolvedPublic

Description

Follow the steps in the "test plan" section of the parent task: T289019: Test Vue 3 migration build with extensions/skins using Vue. The outcome of this task should be that MachineVision runs without errors both under Vue 2 and under the Vue 3 migration build.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

@Catrope I'll post installation and testing notes here early next week; hopefully Monday

For more information about a previous attempt to make MachineVision work with Vue 3, see T289019#7326292. In particular, issue 4622 in Vue affects this extension, and is not yet fixed upstream (I submitted a PR for it, but the upstream maintainers asked me to add a test case, and I haven't had time to do that yet).

I think we can work around the upstream bug by setting compilerOptions: { whitespace: 'condense' } in the component options for components that contain a <transition-group> tag with multiple children, but I haven't tested that yet.

Below are setup and testing instructions.

Extension setup

To set MachineVision up locally, I would recommend following these instructions for setting up WikibaseMediaInfo with Docker, then adding and enabling the MachineVision extension. You might be able to get away with a lighter install including MachineVision, WBMI, WikibaseCirrusSearch, and UniversalLanguageSelector, which are the only hard requirements, since we won't actually be using Wikibase here.

Once you have your environment set up, you can visit Special:SuggestedTags. You will need to be logged in and autoconfirmed (or an admin) to interact with the feature.


Getting images

You won't be able to test with local images without Google Cloud Vision credits. I'd recommend hacking the Vuex getImages action to do the following:

  • Get rid of the API call; you'll just get a database error
  • Comment out the responseIsValid, validItems, and images stuff. We're going to just set images equal to an array of valid image data by visiting the live tool in debug mode, opening up the Vue devtools, loading the Vuex state, and copying the value of images.popular:

image.png (2×2 px, 1 MB)

  • There's a try/catch for setting user stats. Just set both of the values (the second param in each context.commit call) to a number
  • Grab the code that was inside deferred.catch()and keep it, but comment it out for now for testing later
  • Grab the code that was inside deferred.always() and keep it to make sure the pending state is properly disabled

The getImages action will end up looking something like this:

	getImages: function ( context, options ) {
		var queue = options && options.queue ? options.queue : context.state.currentTab;

		ensureTabExists( context.state, queue );

		// Set pending to true and reset error state.
		context.commit( 'setFetchPending', {
			queue: queue,
			pending: true
		} );
		context.commit( 'setFetchError', {
			queue: queue,
			error: false
		} );

		var images = [ (big array of image data) ]

		// Commit the MvImage objects to the state in the appropriate queue
		images.forEach( function ( image ) {
			context.commit( 'addImage', {
				image: image,
				queue: queue
			} );
		} );

		try {
			// Commit user stats and current number of unreviewed personal
			// images into the store.
			context.commit( 'setUserStats', 20 );
			context.commit( 'setUnreviewedCount', 10 );
		} catch ( e ) {
			// Use default state values.
		}

		// Show a generic error message if fetch fails.
		// context.commit( 'setFetchError', {
		// 	queue: queue,
		// 	error: true
		// } );

		// Remove the fetch pending state
		context.commit( 'setFetchPending', {
			queue: queue,
			pending: false
		} );
	}

Add a new tag

To enable local use of the "Add tag" button, add $wgMediaInfoExternalEntitySearchBaseUri = 'https://www.wikidata.org/w/api.php'; to LocalSettings. This will search wikidata for items as you type. You'll probably need something like Firefox's Allow CORS extension enabled for this to work.


Publish tags

This won't work either, but for the purposes of testing the Vue compatibility build and and eventually Vue 3, it doesn't really matter because the UI is effectively the same (a toast notification pops up and you're taken to the next image).

Hi both, I have the MachineVision working locally already ( i have set google and have credits so it is ok).

I will apply the changes suggested by Roan and provide more info (either of success of failure to solve the issue)

@Catrope I have set the suggested config locally to see if it fixes the issue issue, but unfortunately it does not seem to be working as expected (same error is triggered). The code that i used to enable the config is:

( function () {
	var Vue = require( 'vue' ),
		App = require( './components/App.vue' ),
		api = require( './plugins/api.js' ),
		logger = require( './plugins/logger.js' ),
		store = require( './store/index.js' );

	// Remove placeholder UI
	$( document.body ).addClass( 'wbmad-ui-initialized' );
	
	var vueInstance = Vue.createMwApp( $.extend( { store: store }, App ) )
		.use( api )
		.use( logger );

	vueInstance.config.compilerOptions.whitespace = 'condense';
	vueInstance.mount( '#wbmad-app' );
}() );

I debugged the vue sourcecode and the writespace config is being set as expected, but without any result in the Frontend.

Just to also provide you more context, I have tried your PR locally and it does fix our issue and the transition work properly when your patch is applied.

Could you try adding the compilerOptions thing to the component options in the component that contains the <transition-group> tag? I'm sorry I didn't describe it more clearly before. I meant this:

diff --git a/resources/components/SuggestionsGroup.vue b/resources/components/SuggestionsGroup.vue
index fc54bf7..5c41181 100644
--- a/resources/components/SuggestionsGroup.vue
+++ b/resources/components/SuggestionsGroup.vue
@@ -57,6 +57,10 @@ var mapActions = require( 'vuex' ).mapActions,
 // @vue/component
 module.exports = {
 
+       compilerOptions: {
+               whitespace: 'condense'
+       },
+
        components: {
                'mw-button': Button,
                'mw-suggestion': Suggestion

See also this sandbox where I've added that setting (and the error doesn't happen) vs this one where it's commented out (and the error happens).

I have tried your suggestion option to ensure that the condense is just applied to the component, but it is not working as you may expect.

I have debugged the vue source code to make sure that he option condense is loaded properly, and it is.

What it seem to happen, is that the condense is not being applied between the elements that have been rendered (because they are custom component, I am not sure what node they are assigned and if the white space is removed or not (i can debug further if you want, but I have not spent that much time) - See this article for more info https://forum.vuejs.org/t/am-i-using-preservewhitespace-option-of-vue-loader-correctly/62634

What I did realize, is that if I do manually remove the space between the elements, the component actually works fine. So the following works fine:

	<transition-group
		name="wbmad-suggestion-fade"
		tag="div"
		class="wbmad-suggestions-group"
		:class="builtInClasses"
	>
		<mw-suggestion v-for="suggestion in currentImageSuggestions"
			:key="getSuggestionKey( suggestion.wikidataId )"
			:text="suggestion.text"
			:confirmed="suggestion.confirmed"
			@click="toggleTagConfirmation( suggestion )"
		>
			<template v-if="tagDetailsExpanded">
				<label class="wbmad-suggestion__label">
					<span class="wbmad-suggestion__label__text">
						{{ suggestion.text }}
					</span>
					<template v-if="suggestion.alias">
						<span class="wbmad-suggestion__label__separator">–</span>
						<span class="wbmad-suggestion__label__alias">
							{{ suggestion.alias }}
						</span>
					</template>
				</label>
				<p v-if="suggestion.description" class="wbmad-suggestion__description">
					{{ suggestion.description }}
				</p>
			</template>

			<template v-else>
				<label class="wbmad-suggestion__label">{{ suggestion.text }}</label>
			</template>
		</mw-suggestion><div :key="buttonKey" class="wbmad-custom-tag-button-wrapper">
			<mw-button
				icon="add"
				class="wbmad-custom-tag-button"
				:title="$i18n( 'machinevision-add-custom-tag-title' )"
				@click="$emit( 'custom-tag-button-click' )"
			>
				{{ $i18n( 'machinevision-add-custom-tag' ) }}
			</mw-button>
		</div>
	</transition-group>

let me know if you want me to debug further (prob I can compare vue 2 and vue 3 whitespace condensing and see if there is any bug there), or if you are ok for the above fix until your patch is merged upstream ( i can add a comment there)

Ugh, that's annoying. Thanks for investigating this in detail. Your fix removing the space should be OK for now (as long as we add a comment explaining what's going on), until my patch is merged upstream. Upstream asked me to do some more work on my patch too, so I'll do that ASAP.

Change 737347 had a related patch set uploaded (by Simone Cuomo; author: Simone Cuomo):

[mediawiki/extensions/MachineVision@master] Make MachineVision work with the Vue 3 migration build

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

Change 737347 merged by jenkins-bot:

[mediawiki/extensions/MachineVision@master] Make MachineVision work with the Vue 3 migration build

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