Skip to content

Update tests for slower load of async route components #1770

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.

Sign up for GitHub

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

Draft
matthew-white wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Update tests for slower load of async route components #1770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e12c48
mockHttp(): don't put time limit on import of async route components
matthew-white Aug 15, 2026
c813fb7
Load async route components as needed rather than all at once
matthew-white Aug 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions apps/central/src/util/load-async.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,24 @@ loadAsync() has a couple of benefits:
- webpack magic comments will not be repeated across files.
*/

let loadingCount = 0;

const loader = (load) => {
const obj = {
loaded: false,
load: async () => {
const m = await load();
obj.loaded = true;
return m;
loadingCount += 1;
try {
const m = await load();
obj.loaded = true;
return m;
} catch (error) {
// eslint-disable-next-line no-console
if (import.meta.env.NODE_ENV === 'development') console.error(error);

@matthew-white matthew-white Aug 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I understand the finally below: it's needed to decrement loadingCount. But I don't remember the point of this catch. Why is it useful to console.error() the error if we're also re-throwing it? Perhaps #1743 means that this part is unnecessary now.

Also, the use of import.meta.env.NODE_ENV seems a little unusual to me. What is NODE_ENV in the context of testing? Is it 'development'? I assume so, because if not, this line seems kind of irrelevant to the rest of the work of the commit in which it was added.

In most of the app, we use buildMode instead of something like NODE_ENV. I guess buildMode isn't available in the context of this file though, since it doesn't have access to the Frontend container object.

Another thing I'm wondering: why NODE_ENV instead of MODE?

throw error;
} finally {
loadingCount -= 1;
}
}
};
return obj;
Expand Down Expand Up @@ -209,10 +220,23 @@ const loaders = new Map()
'../components/user/list.vue'
)));

export const loadAsync = (name) => loaders.get(name).load;
export const loadedAsync = (name) => loaders.get(name).loaded;
const getLoader = (name) => {
if (!loaders.has(name)) throw new Error(`loader not found for ${name}`);
return loaders.get(name);
};
Comment on lines +223 to +226

@matthew-white matthew-white Aug 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This part wasn't part of the original commit. I just added it when I rebased now. I added it because I removed some error-logging in test/util/http.js as part of resolving a merge conflict.


export const loadAsync = (name) => getLoader(name).load;
export const loadedAsync = (name) => getLoader(name).loaded;



////////////////////////////////////////////////////////////////////////////////
// TEST UTILS

// These functions are exported for use in testing.

export const loadingAnyAsync = () => loadingCount !== 0;

// Exported for use in testing
export const setLoader = (name, load) => {
loaders.set(name, loader(load));
};
5 changes: 0 additions & 5 deletions apps/central/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { expect, should } from 'chai';
import '../src/styles';

import testData from './data';
import { loadAsyncRouteComponents } from './util/load-async';
import { mockLogin } from './util/session';
import { setupLanguages } from './util/i18n';
import './assertions';
Expand All @@ -22,10 +21,6 @@ window.expect = expect;
////////////////////////////////////////////////////////////////////////////////
// HOOKS

// Even if a route is lazy-loaded, load() will need synchronous access to the
// async components associated with the route.
beforeAll(loadAsyncRouteComponents);

enableAutoUnmount(afterEach);
afterEach(() => {
const app = document.querySelector('[data-v-app]');
Expand Down
46 changes: 23 additions & 23 deletions apps/central/test/util/http.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,14 +278,14 @@ import { clone, identity, last, pick } from 'ramda';

import App from '../../src/components/app.vue';

import { loadAsync, loadingAnyAsync } from '../../src/util/load-async';
import { noop } from '../../src/util/util';
import { routeProps } from '../../src/util/router';

import createTestContainer from './container';
import requestDataByComponent from './http/data';
import testData from '../data';
import * as commonTests from './http/common';
import { loadAsyncCache } from './load-async';
import { mockAxiosError, mockResponse } from './axios';
import { mockRouter, setInstallLocation, testRouter } from './router';
import { mount as lifecycleMount, withSetup } from './lifecycle';
Expand All @@ -295,20 +295,13 @@ import { wait, waitUntil } from './util';
const routeResolver = createTestContainer({ router: testRouter() }).router;
const resolveRoute = (location) => routeResolver.resolve(location);
// Returns the components associated with a route. If the route is lazy-loaded,
// any async component will be unwrapped from AsyncRoute.
// any async component will be unwrapped from AsyncRoute. If a component is
// async, only its name will be returned, not the full component.
const routeComponents = (route) => route.matched.map(routeRecord => {
const { asyncRoute } = routeRecord.meta;
if (asyncRoute == null) return routeRecord.components.default;

const m = loadAsyncCache.get(asyncRoute.componentName);
if (m == null) {
// eslint-disable-next-line no-console
console.error(`Component ${asyncRoute.componentName} was not found in the loadAsync() cache.`);
// eslint-disable-next-line no-console
console.error('The loadAsync() cache contains', loadAsyncCache.size, 'entries.');
throw new Error('component not found in loadAsync() cache');
Comment on lines -306 to -309

@matthew-white matthew-white Aug 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think this error-logging existed back when I was working on Vitest: when rebasing these commits, I got a merge conflict in this area of the code. I think it makes sense to remove this error-logging, since loadAsyncCache is removed as part of this PR. However, I've added a little more error handling to loadAsync() in src/util/load-async.js.

}
return m.default;
return asyncRoute == null
? routeRecord.components.default
: { name: asyncRoute.componentName };
});

class MockHttp {
Expand Down Expand Up @@ -384,8 +377,11 @@ class MockHttp {
? containerOption
: createTestContainer(containerOption));

const mount = () => {
const wrapper = lifecycleMount(component, { ...options, container });
const mount = async () => {
const loadedComponent = typeof component === 'string'
? (await loadAsync(component)()).default

@matthew-white matthew-white Aug 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Instead of calling loadAsync() ahead of time in test/index.js (via loadAsyncRouteComponents()), we now do so right before the component definition is needed (before we need to pass it to Vue Test Utils' mount()).

: component;
const wrapper = lifecycleMount(loadedComponent, { ...options, container });

if (throwIfEmit != null) {
const emitted = wrapper.emitted();
Expand Down Expand Up @@ -643,24 +639,24 @@ class MockHttp {
: noop;

try {
const routeBefore = router != null ? router.currentRoute.value : null;
if (this._location != null) await router.push(this._location);
if (this._mount != null) {
this._component = this._mount();
this._component = await this._mount();
// Mounting may have triggered the initial navigation.
if (router != null) await router.isReady();
}
// If there was a navigation, then we need to wait for any async
// components associated with the route to load.
await waitUntil(() => !loadingAnyAsync());

if (this._request != null) {
// If there has been a navigation, then wait for any async components
// associated with the route to load.
if (router != null && router.currentRoute.value !== routeBefore)
await wait();

this._checkStateBeforeRequest();
await this._request(this._component);
}
} finally {
// Wait for any router navigation to finish.
if (router != null) await wait();
await waitUntil(() => !loadingAnyAsync());
Comment on lines +657 to +659

@matthew-white matthew-white Aug 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Here, I think we're waiting for two things. That's why there's both wait() + waitUntil():

  1. Wait for Vue Router navigation hooks to complete. Those should be processed as microtasks, so wait() should give them enough time to complete.
  2. Wait for async route components to load (now longer than a microtask).

// Wait for any responses to be processed.
await wait();
if (pollWork != null) await waitUntil(() => pollWork(this._component));
Expand Down Expand Up @@ -951,7 +947,11 @@ const loadBottomComponent = (location, mountOptions, respondForOptions) => {
const throwIfEmit = `${bottomComponent.name} emitted an event, but it is not expected to do so. In this case, root cannot be specified as false.`;

return mockHttp()
.mount(bottomComponent, fullMountOptions, throwIfEmit)
.mount(
bottomComponent.render != null ? bottomComponent : bottomComponent.name,
fullMountOptions,
throwIfEmit
)
.modify(series => (respondForOptions !== false
? series.respondForComponent(bottomComponent.name, respondForOptions)
: series));
Expand Down
28 changes: 0 additions & 28 deletions apps/central/test/util/load-async.js
View file
Open in desktop

This file was deleted.

Loading