vuejs / core

πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
https://vuejs.org/
MIT License
47.87k stars 8.36k forks source link

Support async injection in async component setup ? #1409

Open AlexandreBonaventure opened 4 years ago

AlexandreBonaventure commented 4 years ago

What problem does this feature solve?

So I don't know if this is a feature or a bug: https://jsfiddle.net/16pyjavn/1/ Feel free to close if that is expected and not in your plan. I know injection is synchronous and should happen in the setup function, now it is not clear to me if it should work with async components.

The use-case I'm trying to solve: Let's say I want to build a component library that conditionally plays with the vue-router. So I'd like to have the vue-router as an optional peerDependencies and import it asynchronously like that:

const RouterHookComponent = defineAsyncComponent(async () => {
  const { useRouter } = await import('vue-router');
  const router = useRouter();
  if (router) // do some stuff 
  ....
  return ...
});

Now this seems a little hacky but I don't see how I could it in other ways since

  1. vue-router extended component guards do not seem to offer an access to the component instance. And I cannot import synchonously import { useRouter } from 'vue-router' since it will break for apps without the router.
  2. I can't use bare inject(routerKey) because the symbol pattern make it a black box (which....don't get me wrong: this is good practice and I love it)
  3. Lastly I could export two different builds so user can choose the one that fits but I would prefer other solutions if there are any
icehaunter commented 4 years ago

I came across the same issue where I'd like to fetch some data in the root component (displaying global loader is fine at that point), and I'd like to provide the fetched data (as it is, essentially, a dictionary) to all child components - they need that dictionary to properly display some data.

AlexandreBonaventure commented 4 years ago

I ended up using this workaround:

const vm = getCurrentInstance();
const router = vm?.ctx?.$router
if (router) ....

Although, ctx is marked as internal API... I wish there is a better way to solve this

somodevo commented 4 years ago

I got a similar use case where I need to fetch some data from an API endpoint when/for initialising our SPA. That data needs to be passed to all children. I also need to do provide(DefaultApolloClient, apolloClient) some time afterwards in the async function. Anyone have an idea or workaround on how to achieve that? πŸ€”

LinusBorg commented 4 years ago

const { useRouter } = await import('vue-router');

Why import the router asyncronously?

  1. If the project uses vue-router in their app, it will already be synchronously imported in their main.js bundle.
  2. If they don't have vue-router installed, this will throw an error
  3. If they have vue-router installed but don't use it so far, you just added needless weight to the Javascript they ship.
AlexandreBonaventure commented 4 years ago

@LinusBorg

  1. If they don't have vue-router installed, this will throw an error

This is exactly what I want to avoid. static imports are not catchable. The goal is to conditionally support vue-router.

LinusBorg commented 4 years ago

@icehaunter provide a ref synchronously and populate it with the data after the asyncs fetch operation has finished.

@somodevo if you need to fetch data before you initialize your app, then don't initialize your app before you got the data. Different problem then what this issue is about.

This is exactly what I want to avoid. static imports are not catchable. The goal is to conditionally support vue-router.

@AlexandreBonaventure We can use require() in a try/catch block for these things in most build tools, but I agree that this is not very future-oriented, as we (as in: the JS ecosystem) try and move towards ESModules and away from commonjs crutches.

Concerning your approach in the original post, I was a bit glossing over the fact that you used an async component. Was the idea to return different implementations depending wether or not the router is available, or what do you have in mind?

I suppose it would be technically possible to have access to injections of the currentInstance for which this async component is currently being resolved, but not sure yet if this introduces any tricky edge cases or foot guns.

icehaunter commented 4 years ago

@LinusBorg That is what I ended up doing. This approach, however, has a couple of drawbacks. Mainly - I cannot rely on Suspense to correctly display an application-wide loader while data required for the application is loading

somodevo commented 4 years ago

@LinusBorg - Thanks I'll try that and see if/how I can decouple the Vue dependencies from the other initialisation logic. However, I wanted to do it in the Vue context because I wanted to use Suspense to show a loading screen in the meantime. That's why I think my case is related to this issue.

LinusBorg commented 4 years ago

@icehaunter I don't see why that would be the case? You're being a bit unspecific here, so I'm not sure how to answer. A real example would be helpful.

@somodevo same applies to you.

somodevo commented 4 years ago

Once I got it working or not I will share a fiddle/codesandbox. Thanks for taking the time πŸ‘

znck commented 4 years ago

Can't you use top level import and depend on tree shaking in end application?

AlexandreBonaventure commented 4 years ago

@icehaunter @somodevo Sorry but it looks like your scenarios are a little bit different than mine, and can be dealt with current API.

@LinusBorg I'm mostly using vite these days so require is a showstopper @znck Not sure to get what you mean. Tree-shaking will not happen if the module itself is not found, it will throw an exception (In this case, I want to conditionnaly support vue-router if the end application is using it).

somodevo commented 4 years ago

@AlexandreBonaventure I guess @icehaunter and my use case is more closely to #1826 which got closed as a duplicate of this issue. However, I'll provide an example as mentioned earlier when I continue working on evaluating Vue for our SPA rewrite.

LinusBorg commented 4 years ago

@AlexandreBonaventure I think what @znck means is that you should make vue-router a dependency of your component library. Wether or not the project that consumes your library actually uses vue-router, you import it and use it's useRoute method - which means it's a dependency of your project.

If the consuming project is using a proper build process, everything but the useRoute implementation should be tree-shaken from the final bundle, if the consuming project itself doesn't use VueRouter.

This poses a problem to projects not using a bundler, still thinking about that part ...

KaelWD commented 4 years ago

If vue-router provided 'vue-router' instead of a symbol and had router.useRouter you could do inject('vue-router')?.useRoute() without importing anything.

AlexandreBonaventure commented 4 years ago

@LinusBorg correct me if I'm wrong but tree-shaking happens after module resolution. So I don't think it would help at all. If a dependency is missing -> module resolution fails -> build fails before tree-shaking.

Wether or not the project that consumes your library actually uses vue-router, you import it and use it's useRoute method - which means it's a dependency of your project.

Ideally, I'd rather specify vue-router as an external in the build config and an optional dependency in the package management side of things. Because, to me that's really what this is.

LinusBorg commented 4 years ago

correct me if I'm wrong but tree-shaking happens after module resolution. So I don't think it would help at all. If a dependency is missing -> module resolution fails -> build fails before tree-shaking.

By making vue-router a dependency of your component library, it will never be missing. and tree-shaking will remove anything but these 3 lines:

export function useRouter(): Router {
  return inject(routerKey)!
}

That would work with an external config when used in a project with a bundler, but would require the inclusion of the whole VueRouter package when used directly in a Browser with a script tag, (which I tried to say with my next paragraph) - which is of course undesirable.

mcrio commented 3 years ago

@AlexandreBonaventure I guess @icehaunter and my use case is more closely to #1826 which got closed as a duplicate of this issue. However, I'll provide an example as mentioned earlier when I continue working on evaluating Vue for our SPA rewrite.

I think @somodevo has a valid point here. If not related to this issue then maybe #1826 should remain open? Example: https://jsfiddle.net/jnikola/umswkrhf/26/

paraboul commented 3 years ago

Joining @somodevo here,

My use case (similar to closed #1826) :

How it's currently done :

Parent view

    export default {
        name: "ParentView",
        setup() {
            const { loadUser } = useUser();

            /* Load current user so it's available to children view */
            /* provide() can't be used in `async setup` so we have to provide a Promise and children have to await it */
            provide("user", loadUser());
        }
    }

Nested view

    export default {
        name: "ChildView",

        async setup() {
            const user = await inject("user");
            return {
                user
            }
        }
    }

It would be more convenient to have the parent view be async itself rather than having any children view awaiting for the injected property. It would also allow the parent view to have the await'd data available for itself.

LinusBorg commented 3 years ago

I think that should be kind of achievable by providing a ref before running any async operations:

export default {
        name: "ParentView",
        async setup() {
            const user = ref()
            provide('user', user)
            user.value = await useUser()

            // .... 

        }
    }

if you don't want to use a ref, an empty reactive object might also work if you don't need object identify:

export default {
        name: "ParentView",
        async setup() {
            const user = reactive({})
            provide('user', user)
            const _user = await useUser()
            Object.assign(user, _user)

            // .... 

        }
    }
paraboul commented 3 years ago

@LinusBorg indeed, although there are drawbacks in both cases.

In your first example, user now contains a ref inside another ref (see edit below), and so needs to be unref'd in ChildView, which is kind of confusing. In the second one, as you said, it no longer keeps track of the original identity.

Although I'm not sure how it could find its currentInstance (by optionally passing the context maybe?), in theory, provide() should be able to work since children components are not loaded until after the parent component is resolved.

Edit Sorry for the confusion, in my original example loadUser() is await'ing a reactive property.

vitorrd commented 4 days ago

My particular use case is that of dynamically importing specific composables for different application areas. Each area may define its own composable, and writing a single file with a gigantic switch would be bad design, as would be writing wrapper components to resolve the imports under a Suspense boundary. Since I can't wait for a decision to be made, I'm moving on with an alternative, but thought I'd write here since what I need to do is, as far as I know, currently impossible under the current architecture, and as much as it's a bit of an exception, dynamically importing composables seemed like enough of a fair use case to be reported.