Fix useModelMigration in async components

[baltitenger]

It was using getCurrentInstance() which, as I understand it, isn't part of vue's public API anymore and isn't guaranteed to work in every situation.

Makes call site a bit uglier, but I don't think there's a better way to do it.

☑️ Resolves

• Fix useModelMigration broken in async components #6607

🏁 Checklist

• ⛑️ Tests are included or are not applicable
• 📘 Component documentation has been extended, updated or is not applicable
• 3️⃣ Backport to next requested with a Vue 3 upgrade

[ShGKme]

It was using getCurrentInstance() which, as I understand it, isn't part of vue's public API anymore

It is a part of exposed Advanced API. It was called internal in the past indeed. Because it returns ComponentInternalInstance which is internal API and can be changed. However, Vue 2.7 is EOL so the result will never change. Also, Vue 2 didn't have a separation to ComponentInternalInstance and ComponentPublicInstance - so the result is a public instance.

and isn't guaranteed to work in every situation.

Could you clarify on a case when it isn't supposed to work?

[codecov]

Codecov Report

Attention: Patch coverage is 73.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 43.11%. Comparing base (4c77334) to head (2e98a83).
Report is 222 commits behind head on master.

Files with missing lines	Patch %	Lines
src/components/NcActionRadio/NcActionRadio.vue	50.00%	1 Missing ⚠️
...ents/NcActionTextEditable/NcActionTextEditable.vue	50.00%	1 Missing ⚠️
...ts/NcCheckboxRadioSwitch/NcCheckboxRadioSwitch.vue	0.00%	0 Missing and 1 partial ⚠️
src/components/NcSelectTags/NcSelectTags.vue	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6608      +/-   ##
==========================================
+ Coverage   42.50%   43.11%   +0.60%     
==========================================
  Files         156      161       +5     
  Lines        4028     4108      +80     
  Branches     1036     1070      +34     
==========================================
+ Hits         1712     1771      +59     
- Misses       2200     2218      +18     
- Partials      116      119       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[baltitenger]

Could you clarify on a case when it isn't supposed to work?

I'm not actually sure, I think I read somewhere that it's only guaranteed to work from lifecycle events but setup is one, so I don't know why it wouldn't work in this situation. But I did observe it returning null so clearly it can happen (see the mentioned issue).

Another thing, I just noticed that I forgot to update the jsdoc comment for useModelMigration, but I'm not sure how to type these two new parameters. (plus typescript-language-server seems to ignore typing information in jsdoc, I guess in typescript you're supposed to have it inline)

[ShGKme]

I'm not actually sure, I think I read somewhere that it's only guaranteed to work from lifecycle events but setup is one, so I don't know why it wouldn't work in this situation

getCurrentInstance only supposed to work when there is the current instance - in the setup context. Same as all the other Composition API function and composables.

And the setup function has the setup context. Otherwise not only this function wouldn't work here, but any composable and even reactive effect wouldn't be cleared after component is unmounted and etc.

But I did observe it returning null so clearly it can happen (see the mentioned issue).

I'd guess that you test not with installed @nextcloud/vue, but with npm link to local @nextcloud/vue. In this case there are 2 copies of Vue in different node_modules. And Vue doesn't support it in general.

In this case, you need to either use npm pack instead of npm link to have a complete reproduction of installed package, or resolve vue as alias to app's node_modules.