are we doing this consistently in a way that is straightforward to understand?
if a refactor can be done within the timebox, this can be done as a part of this task
if there is more to be done, then a summary of the investigation should be made
Timebox: 8 hours (2 hours used so far)
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | None | T394621 [MEX] Mobile Editing Experience of Items Project | |||
| Open | None | T400128 [MEX] M3 - Create editing functionality for statements on Items on mobile | |||
| Open | None | T400129 [MEX] M3.1 edit function for a simple text input | |||
| Resolved | karapayneWMDE | T401700 [MEX] M3.1.1 First Pass - Edit Functionality Simple text input (string) | |||
| Open | None | T401702 [MEX] M3.1.2 Functional- Edit Functionality Simple text input (string) | |||
| Resolved | karapayneWMDE | T405453 [MEX] M3.1.2 - [Spike] review use of options and composition API in publishing |
I think wikibase.wbui2025.addStatementButton.vue has a good example of the usage of the setup() function within a component that mostly uses the options API:
setup( props ) { const propertySelectorVisible = ref( false ); return { cdxIconAdd, propertySelectorVisible }; }
It’s also possible to write this as data instead:
data: () => ( { cdxIconAdd, propertySelectorVisible: false } ),
For propertySelectorVisible, I’d say this is 100% idiomatic; for cdxIconAdd, it’s a bit awkward (it’s not exactly data, more “something we need available in the template”), but might still be the best non-setup() option. I searched for some alternatives but found nothing that really convinced me (for example, there’s a constants plugin, but we don’t actually need the value frozen – in this case it’s a string anyways). The following might be the least worst alternative to putting it in data:
data: () => ( { propertySelectorVisible: false } ), created() { this.cdxIconAdd = cdxIconAdd; }
There’s one other usage of setup(), which can be seen e.g. in wikibase.wbui2025.statementSections.vue, and which can be very straightforwardly replaced with methods:
diff --git i/repo/resources/wikibase.wbui2025/wikibase.wbui2025.statementSections.vue w/repo/resources/wikibase.wbui2025/wikibase.wbui2025.statementSections.vue index 8274fd9503..10fbc45d08 100644 --- i/repo/resources/wikibase.wbui2025/wikibase.wbui2025.statementSections.vue +++ w/repo/resources/wikibase.wbui2025/wikibase.wbui2025.statementSections.vue @@ -41,10 +41,8 @@ module.exports = exports = defineComponent( { required: true } }, - setup() { - return { - concat - }; + methods: { + concat } } ); </script>
wikibase.wbui2025.propertySelector.vue has a pretty long setup function, but I think it mostly falls into the same categories: several of the variables (all the ref()s) could be data; the functions could be methods; addButtonDisabled is literally computed (not seen above, but a straightforward conversion); headingMessage could be either computed or “constant data”; menuConfig is “constant data”; languageCode is either “constant data” or could even be defined outside the component definition, as it’s not used in the template, only in the JS code.
I think all the other setup functions are instances of the same behavior. Everything that’s returned from a setup function can in principle be moved into data, methods, computed, or “constant data” either in data or in a created() hook as shown above.
IMHO, which approach we should go for is up for debate. To me both APIs, and also mixing them, seem fine, so I’d be okay with leaving this up to “developer choice”. But if we want to stick more strongly to the options API, we can also do this. Conversely, we can certainly go more towards the setup() function as well, though I think we won’t be able to write <script setup> (it’ll still be a setup() function as an option in the component definition).
Change #1191410 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Replace some straightforward usages of setup()
Change #1191411 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] WIP: Port StatusMessage to options API
Change #1191421 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Port PropertySelector to options API
I'm in a similar position - I feel like both APIs are fine and mixing them isn't a problem. But reviewing the patches and reading more into it has also helped me separate out the two approaches in my head. I would support us moving over time to the options API for new code. For existing code, while everything is quite fresh with wbui2025, I think it probably makes sense to port the files over as we edit them.
Change #1191410 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Replace some straightforward usages of setup()
Change #1191421 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Port PropertySelector to options API
Change #1191697 had a related patch set uploaded (by Sadiya.mohammed13; author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Port PropertySelector to options API
Change #1191697 abandoned by Arthur taylor:
[mediawiki/extensions/Wikibase@master] Port PropertySelector to options API
Reason:
See patch I987ea3c052 with alternative commit message
Change #1191411 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Port StatusMessage to options API
Change #1194580 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Fix comment
Change #1194580 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Fix comment