core icon indicating copy to clipboard operation
core copied to clipboard

feat(runtime-core): app.onUnmount() for registering cleanup functions (close: #4516)

Open LinusBorg opened this issue 4 years ago • 5 comments

This PR adds a new feature ~~for Plugins~~. Library Authors can now register a callback function using app.onUnmount() which will be called immediately after the app has been unmounted with app.unmount()

Details

  • the function doesn't receive any arguments. the app instance should be available from the closure of the install function
  • if the install function returns anything but a function with zero arguments, it will be ignored.This this should ensure a reasonable guard against unintentional effects from plugins accidentally returning something before this feature was added.
// ✅ Valid
function install (app: App) {
  function cleanupSomeSideeffect() { /* ...*/ }

  app.onUnmount(cleanupSomeSideffect)
}

Open Questions (from previous iteration)

  • [x] is the argument length guard a good idea? => no.
  • [x] Can we accept that we might break rare edge cases where plugins currently return a function with zero arguments that is not meant to be called after unmount? => no
  • If the answer to the previous question is "no" - Should we instead provide an API on app like app.onUnmount()? => yes

close: #4516

LinusBorg avatar Sep 17 '21 08:09 LinusBorg

Size report

Path Size
vue.global.prod.js 46.19 KB (+0.08% 🔺)
runtime-dom.global.prod.js 30.55 KB (+0.11% 🔺)
index.js 18.05 KB (+0.17% 🔺)

github-actions[bot] avatar Sep 17 '21 08:09 github-actions[bot]

Some thoughts:

  1. implicit checking function.length can lead to bugs that are difficult to troubleshoot (for plugin authors).
  2. Murphy's law. 🤯
  3. at present, the general practice of registering unmount hooks is to replace the original app.mount function; I think app.onUnmount is better, since it's more in line with the existing usage(, and more elegant).

unbyte avatar Sep 19 '21 16:09 unbyte

I also think the length check seems a bit unnecessary/incomplete.

app.onUnmount is safer and also more generic as it can be used even in non-plugin scenarios.

yyx990803 avatar Sep 21 '21 17:09 yyx990803

I agree with your comments and will adjust the PR accordingly

LinusBorg avatar Sep 21 '21 17:09 LinusBorg

Hey 👋 any news here?

aantipov avatar Jan 21 '22 16:01 aantipov

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 94.1 kB (+3.34 kB) 35.5 kB (+1.04 kB) 32 kB (+916 B)
vue.global.prod.js 151 kB (+3.44 kB) 54.8 kB (+1.13 kB) 48.9 kB (+936 B)

Usages

Name Size Gzip Brotli
createApp 54 kB (+3.2 kB) 20.9 kB (+1 kB) 19 kB (+879 B)
createSSRApp 57.3 kB (+3.2 kB) 22.2 kB (+999 B) 20.2 kB (+870 B)
defineCustomElement 56.3 kB (+3.2 kB) 21.6 kB (+1.02 kB) 19.6 kB (+856 B)
overall 67.8 kB (+3.29 kB) 25.9 kB (+995 B) 23.4 kB (+852 B)

github-actions[bot] avatar Oct 20 '23 15:10 github-actions[bot]