Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alternative to confusing async component loading behavior #193

Closed
AaronBeaudoin opened this issue May 4, 2021 · 14 comments · Fixed by #212
Closed

Alternative to confusing async component loading behavior #193

AaronBeaudoin opened this issue May 4, 2021 · 14 comments · Fixed by #212

Comments

@AaronBeaudoin
Copy link

AaronBeaudoin commented May 4, 2021

The Problem

In Nuxt components v2, components are dynamically imported with async. This change was a nightmare for me because it result in several issues which took the better part of several days to find the cause of.

Hard to Find Bug Resulting from the Problem

Here is the issue I had created on the Nuxt repository the first time I ran into the problem:
nuxt/nuxt#8981

The solution required me to write extra code in my custom component because I needed to ensure that $el existed before I could initialize a gsap animation. It was very counter-intuitive for me to not be able to assume that $el was populated inside of the mounted() hook, instead having to write a conditional inside the updated() hook to check if the element was populated each time. Honestly not a great solution in my opinion.

Also, this only broke I after I upgraded to v2 of Nuxt components, and unfortunately the migration guide was of no help to me because it didn't explain that components being automatically made global also meant they were being imported dynamically. I was totally in the dark on that detail.

Another Different Hard to Find Bug Resulting from the Problem

Today I just ran into another problem which I eventually discovered was resulting from async component loading. This time, my styling broke because I relied on the assumption that an id attribute would be present, but it wasn't due to this same async component loading. Nothing about this behavior was intuitive to me.

Here is a simple Code Sandbox that shows the second problem in action:
https://codesandbox.io/s/boring-bassi-et0fb?file=/layouts/default.vue

What I'd Like to See

Documentation for Effects of Async Component Loading

First, we need a clear part of the documentation that explains the downsides of the async component loading so people don't spend hours trying to debug the source of weird issues. I imagine that 99% of Vue users expect $el to always exist in the mounted() hook, and id attributes to never disappear without a clear cause. Any behavior that changes this makes for a very dangerous default, in my humble opinion.

Official Support for loader: true

In the issue I linked above, @pi0 revealed that we can set loader: true to re-enable v1 behavior. However, he also stated that this is "not recommended nor guaranteed to always work". I propose that this solution be made officially supported for those who wish to not lose sleep at night.

Somewhere in nuxt.config.js to Exclude Specific Components from Async Loading

Additionally, provide a way to list components in nuxt.config.js that should not be loaded in an async manner.

Why not just import components manually? Random importing certain components here and there in a codebase to force them to not be loaded async is confusing, because there is not immediately any apparent reason why one component is manually imported and another isn't, unless clear comments are left. Anyone could remove the component import thinking they are just cleaning things up and end up breaking the application. Also, manually importing components defeats part of what I love about Nuxt components, which is cleaner code due to not having to import them manually.

TL;DR / Summary

  • Clearly document the behavior changes when using async components. Even if this is just a link to some existing Vue documentation for what happens when you use async components, it would be extremely helpful because I'll bet a lot of more serious users are running into issues resulting from async component loading as a default and scratching their heads.
  • Provide an officially supported way to not use async components. I understand why you guys started using them, but I've already run into two situations where they make things harder instead of easier, and I'm willing to bet many others have as well.
    • Make using loader: true officially supported, rename it to async: false or something like that so it's clear what it does, and document how and when to use it.
    • Provide a property with the name staticComponents or excludeAsync something like that where paths to components or maybe even entire directories of components can be specified to be excluded from being loaded dynamically with async.

Bottom line, let's not force people to stop using the mounted() hook or manually import components (negating the benefit of Nuxt components) to make initial component state consistent in their applications.

Thanks so much for your hard work on Nuxt, and I appreciate your consideration of my suggestion.

@harlan-zw
Copy link

harlan-zw commented May 13, 2021

This is also tripped us up. We had quite a number of components doing DOM / $ref calculations and modifications on the mounted() hook. Some of the operations are expensive so having them run on updated() could be a problem, we would need to do a full audit.

I think the direction of these changes is good but we're missing the documentation around the side-effects of v2. We are also missing documentation for an easy "escape hatch" for this behaviour for larger projects loader: true.

@KaelWD
Copy link

KaelWD commented May 13, 2021

I don't understand why they all have to be async imports in the first place. The file generated for dev looks like this:

import Vue from 'vue'
import { wrapFunctional } from './utils'

const components = {
  MyComponent: () => import('../../app/components/MyComponent.vue' /* webpackChunkName: "components/MyComponent" */).then(c => wrapFunctional(c.default || c)),
}

for (const name in components) {
  Vue.component(name, components[name])
  Vue.component('Lazy' + name, components[name])
}

But it could just as easily generate static imports for non-lazy components:

import Vue from 'vue'
import { wrapFunctional } from './utils'

import { MyComponent as _MyComponent } from '../../app/components/MyComponent.vue'

const components = {
  MyComponent: _MyComponent,
  LazyMyComponent: () => import('../../app/components/MyComponent.vue' /* webpackChunkName: "components/MyComponent" */).then(c => wrapFunctional(c.default || c)),
}

for (const name in components) {
  Vue.component(name, components[name])
}

Turns out you already have this anyway in .nuxt/components/index.js but it isn't used in development??? The dev file could literally just be

import * as components from './index'

for (const name in components) {
  Vue.component(name, components[name])
}

@pi0
Copy link
Member

pi0 commented May 13, 2021

Hi, @AaronBeaudoin thanks for explaining your concerns.

For sure documentation needs to be improved for the migration guide. (nuxt/website-v2#1244, nuxt/website-v2#1434). Meanwhile, I will try to make the readme also more clear.

Why loader is disabled in development mode?

Fixing development performance issues (see #18) and edge cases (see next section) which was much wider than async import edge cases.

Why it is stated loader: true is not guaranteed to work?

It doesn't mean this option is unofficial nor that will be removed in nuxt2. But backs to edge cases the loader simply cannot detect components. Most notably:

  • When using dynamic component names (<component is="">)
  • When not using .vue SFC files (HTML, custom syntax or render function in js/ts)
  • Using nuxt-vite that doesn't support loader

Why async components have such issues?

Unfortunately, they are mostly known vue2 issues waiting to be released or fixed. See vuejs/vue#11963 and vuejs/vue#11837. Also see nuxt/nuxt#8981 (comment) regarding mounted/ref issue.

So can I use loader: true?

Absolutely! Considering performance and usage edge cases are fine.

@pi0
Copy link
Member

pi0 commented May 13, 2021

@KaelWD Seems a good idea to try. There is only one problem that was in contrast to our future development plans (lazy compilation support via vite). We can at least enable this behavior under a flag.

@KaelWD
Copy link

KaelWD commented May 13, 2021

Another edge case I just found with loader: true is that if you have a component using Vue.extend, it will always use the globally registered async components instead of the injected non-async ones. This is because an extended component.exports.options.components now extends the prototype of $root.$options.components.

lazy compilation support via vite

I feel like I'd probably turn that off anyway. Having everything be async is probably fine for really basic apps, but as soon as you start touching the DOM manually or using $nextTick() it becomes completely unusable. Not to mention that it doesn't match the production behaviour at all, so you might get it working perfectly and then everything falls apart once you deploy it.

@pi0
Copy link
Member

pi0 commented May 13, 2021

As i mentioned most of async component edge cases back to known issues of vue@2. By using nuxt you are already using async components (pages). Also loading: true should make dev setup same as production (please use new issue with reproduction if it is not)

is that if you have a component using Vue.extend, it will always use the globally registered async components instead of the injected non-async ones.

Also for this an issue with reproduction would be nice 😊

@AaronBeaudoin
Copy link
Author

I feel like I'd probably turn that off anyway. Having everything be async is probably fine for really basic apps, but as soon as you start touching the DOM manually or using $nextTick() it becomes completely unusable. Not to mention that it doesn't match the production behaviour at all, so you might get it working perfectly and then everything falls apart once you deploy it.

If you ask me, this is the heart of the discussion. With async components it's very tough to reliably do anything that depends on the DOM, which most large applications have to do in one way or another. And as long as behavior in development doesn't match production, we're basically asking for unexpected bugs in our production applications.

I can see how async components would be a better solution if they didn't have Vue issues, but considering all the bugs with async components that have yet to be resolved in Vue, it seems to me that they are simply too unstable to be the default behavior at the moment. The key idea I have to disagree with is that the loader edge cases are much wider than the async component edge cases. The issues with async components are huge, in my opinion.

It doesn't mean this option is unofficial nor that will be removed in nuxt2. But backs to edge cases the loader simply cannot detect components. Most notably:

  • When using dynamic component names (<component is="">)
  • When not using .vue SFC files (HTML, custom syntax or render function in js/ts)
  • Using nuxt-vite that doesn't support loader

These loader edge cases all seem like very predictable trade-offs to me. It makes perfect sense to me that the loader doesn't work in the scenarios you listed. Personally, I can easily live with these trade-offs. However, none of the issues I'm seeing pop up as a result of using async components seem intuitive to me. They were very difficult to debug and contradicted the documented behavior I expected from Vue components.

I'd suggest that the trade-offs of async components are far worse than the trade-offs of using a loader, because from what I understand, the loader trade-offs are simply limitations, but the async component trade-offs are bugs. That makes the loader trade-offs expected, while the async component trade-offs are generally unexpected, but that's just my two cents.

@pi0
Copy link
Member

pi0 commented May 13, 2021

I fully understand your thoughts @AaronBeaudoin and it is unfortunate hearing inconveniences you occurred. But the fact is that there is no single choice fitting for all and choosing to disable loader was based on amount of issues reporting that components is not working with not easy way to debug either or even fix some (like only solution for jsx was explicitly using global/ dir)

Choosing loader: false for default was ensuring we use an option that is more futuristic for supporting vite and fixing root causes in vue itself and meanwhile fixing dx issues originating from components module itself (so that once root cause is fixed we don't need breaking changes again) And it probably makes sense to enable it for development if project is depending on features that involve dom access.

@KaelWD mentioned a good idea that when loader is disabled and during development, we can use non async imports. It makes issue appearing again for supporting lazy compiling but since it is not a case as of now, adding a flag would be nice (we can enable it bg default after trying it for a while for tradeoffs) .

@Markiz9999
Copy link

Can I somehow check that all asynchronous components have been loaded in the updated event?

@AaronBeaudoin
Copy link
Author

It's frustrating that I need to come back here a 3rd time, but here I am. I just wasted an entire day of work trying to find the source of a problem and it ended up being this exact same cause again.

Please see the below CodeSandbox containing a minimal test case and think about how, in a complex project, you would have figured out what the core problem is. I guarantee a new user starting off with Nuxt would just uninstall and move on to React if they ran into an issue like this right off the bat.

https://codesandbox.io/s/relaxed-hofstadter-4jdoy

Simple Explaination of Issue

In the attached example, a minimal index.vue page includes nothing but a <Test> component as it's root element. The <Test> component itself contains nothing but a <div> whose contents is a <slot>. Back in the index.vue page, the slot contains a <div> with the text contents of the path of an image, and below it is an <img> element displaying the actual image.

In the JS for the index.vue page, the path to the image is an image root property in the data of the page component. It is first set in asyncData() and then updated in the mounted() hook. Now here is where everything goes wrong:

When the page component first renders, the path is to the mountains_01.jpg file, which was set in asyncData(). The text for the path and the image in the template both render correctly at this point. However, after the image is updated to the mountains_02.jpg file in the mounted() hook, only the text for the path actually changes. The image in the <img> tag remains the same, and does not update to show mountains_02.jpg like it should. Check what both of the images look like in the project tree to confirm this. Also, if the issue does not occur at first, try reloading the page in the preview window.

How would one resolve this? I tried over a dozen ideas over the course of an entire day. In the end, the only way to resolve it was to either use loader: true or ensure all components through the tree up to the <img> element were loaded explicitly. The following is just my opinion, but that's insane. I am seriously considering ditching Nuxt and the Vue ecosystem altogether for Next and React.

@lucadalli
Copy link

lucadalli commented Jun 24, 2021

@AaronBeaudoin I return to this issue every few days hoping for signs of progress. I have been using the Nuxt framework intermittently (work-permitting) for the past 3 years and this is the only significant regression I've had to deal with.

I went from loving nuxt/components to absolutely hating it due to the behavior described above. My application needs guarantees about the mounting order of sync components and losing that guarantee has been a nightmare.

Does Next offer an equivalent of nuxt/components which works as expected? Instead of jumping ship altogether and using Next, have you considered disabling nuxt/components and suffering the manual imports? That's what I'm considering atm and it seems like a decent trade-off.

@pi0
Copy link
Member

pi0 commented Jun 24, 2021

Hi, @lucadalli sorry to hear about that inconvenience. We are busy working on nuxt3 that's why sometimes features like this are delayed but PRs are always welcome.

By regression, you mean async component issues in development? You can work around it by using loader: true to be the same behavior as was before 2.15.

@lucadalli
Copy link

@pi0 I understand that your plates are full.
Yes, that's what I meant. I had tried enabling loader: true after updating to components v2 but some erratic behavior which was not present prior to the update still persisted.

In fact, I don't have it enabled right now because it did not fix the issues I was facing. I will try re-enabling it. It should at least get me closer to my desired behavior.

@pi0
Copy link
Member

pi0 commented Jun 24, 2021

Thanks for explaining. In the meantime, I will try to implement #193 (comment) for using non-async imports in dev (without vite there is no perf improvement in dev).

Still sharing more details about your current issue with loader would be appreciated.

@pi0 pi0 mentioned this issue Jul 8, 2021
4 tasks
@pi0 pi0 closed this as completed in #212 Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants