Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

feat(bridge, nuxt3): mock vue-demi #1849

Merged
merged 10 commits into from
Nov 15, 2021
Merged

feat(bridge, nuxt3): mock vue-demi #1849

merged 10 commits into from
Nov 15, 2021

Conversation

antfu
Copy link
Member

@antfu antfu commented Nov 11, 2021

πŸ”— Linked issue

Close nuxt/bridge#201

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

I think it's safe to only mock for bridge since in Vue 3 vue-demi is simply some redirection without side effects.

Todo: (@pi0)

  • refactor alias in nuxt3
  • mock @vuejs/composition-api for nuxt3
  • #app and auto imports from vue-demi utils (isVue2 and isVue3)

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@netlify
Copy link

netlify bot commented Nov 11, 2021

βœ”οΈ Deploy Preview for nuxt3-docs canceled.

πŸ”¨ Explore the source changes: 9e35588

πŸ” Inspect the deploy log: https://app.netlify.com/sites/nuxt3-docs/deploys/61923582e53c6a000757d53a

@pi0
Copy link
Member

pi0 commented Nov 11, 2021

It would be nice to implement for Nuxt3 before closing linked issue since:

  • vue-demi as I could understand uses a postinstall hook to detect version and copy impl which can go wrong in some edge cases (like when using both nuxt2 and nuxt3 in monorepo) while in Nuxt 3 during build we are absolutely sure it is vue3 or vue2
  • Same flags can be available even as global auto imports since it is going to be a common asset for module authors and vue-demi idea is already amazing only we could make it more stable in Nuxt's context

(We can do above in another PR πŸ‘)

@antfu
Copy link
Member Author

antfu commented Nov 11, 2021

Added the mock for Nuxt 3, not sure what's the best place to set the alias, feel free to modify.

Also converting to draft for now, I will need to do some tests for it.

@antfu antfu marked this pull request as draft November 11, 2021 09:03
@pi0 pi0 changed the title feat(bridge): mock for vue-demi feat(bridge, nuxt3): mock vue-demi Nov 11, 2021
@pi0 pi0 assigned pi0 and antfu Nov 11, 2021
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please ping when done testing @antfu (we can always improve by trying edge releases too :D)

@antfu
Copy link
Member Author

antfu commented Nov 11, 2021

Love your changes!

However I guess this is a bit tricky, in my testing, our mock does not take effects for @vueuse/core since it's been pre-optimized by vite (so the aliasing is not working), would only work when I exclude them:

optimizeDeps: {
  exclude: [
    '@vueuse/core',
    '@vueuse/shared',
    'vue-demi'
  ],
}

I don't think it's flexible as we can't possibly list all the packages that use vue-demi. Some idea:

  • Disable the bundling feature of vite, which would be fine for most of the modern packages that only ship one single js file, but would cost the perf when user importing things like lodash.
  • We let this to library integration library to extend the vite config then exclude themselves (I can do in VueUse and I think Pinia could do as well).

@pi0
Copy link
Member

pi0 commented Nov 11, 2021

Good points @antfu!

Well indeed this mocking solution only works for transpired dependencies to mitigate compatibility issues and certainly, we cannot prevent all problematic cases. (however, there is now a escape path :D)

It makes sense to include vue-demi to optimizeDeps.exclude array by default and let module authors add by need.
(Related: Basically, the way to config a library being inlined, is by pushing to options.build.transpile that affects all different places to ensure lib is bundled and transpired. We shall add build.transpile to optimizeDeps.exclude by default I think)

Regarding pinna's Nuxt module, I made a rewrite PR (vuejs/pinia#791) that splits the vue2 plugin from vue3. But I can see for @vueuse/* case, it is almost impossible however I can see you are prebundling so I guess the impact of including it in optimizeDeps.exclude would be low.

@antfu antfu marked this pull request as ready for review November 12, 2021 01:44
@antfu
Copy link
Member Author

antfu commented Nov 12, 2021

That makes sense. Then I think we could merge it and I will do the exclusion on VueUse.

@pi0 pi0 merged commit f298386 into main Nov 15, 2021
@pi0 pi0 deleted the feat/bridge-mock-vue-demi branch November 15, 2021 10:25
@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mock vue-demi
3 participants