feat(warn): catch non exposed loaders through NavigationResult

Close #759

Author: posva

src/data-loaders/defineLoader.spec.ts

@@ -253,5 +253,97 @@ describe(
         expect(nestedPending.value).toEqual(false)
       })
     })
+
+    describe('warns', () => {
+      // this might happen if the user forgets to export a loader
+      // https://github.com/posva/unplugin-vue-router/issues/759
+      it('warns if a NavigationResult is committed', async () => {
+        const router = getRouter()
+        const l1 = mockedLoader({ key: 'my-key' })
+        router.addRoute({
+          name: '_test',
+          path: '/fetch',
+          component: defineComponent({
+            setup() {
+              const { data } = l1.loader()
+              return { data }
+            },
+            template: `<p>{{ !!data }}</p>`,
+          }),
+          meta: {
+            // oops! no loader exported
+            loaders: [],
+          },
+        })
+        const wrapper = mount(RouterViewMock, {
+          global: {
+            plugins: [
+              [
+                DataLoaderPlugin,
+                {
+                  router,
+                },
+              ],
+              router,
+            ],
+          },
+        })
+
+        l1.spy.mockResolvedValueOnce(new NavigationResult('/?redirected=true'))
+
+        await router.push('/fetch')
+        await vi.advanceTimersByTimeAsync(1)
+        expect(router.currentRoute.value.fullPath).toBe('/fetch')
+        // no data
+        expect(wrapper.text()).toBe('false')
+
+        expect('NavigationResult').toHaveBeenWarned()
+        expect('key: "my-key"').toHaveBeenWarned()
+      })
+
+      it('warns if a NavigationResult is throw on a non exposed loader', async () => {
+        const router = getRouter()
+        const l1 = mockedLoader({ key: 'my-key' })
+        router.addRoute({
+          name: '_test',
+          path: '/fetch',
+          component: defineComponent({
+            setup() {
+              const { data } = l1.loader()
+              return { data }
+            },
+            template: `<p>{{ !!data }}</p>`,
+          }),
+          meta: {
+            // oops! no loader exported
+            loaders: [],
+          },
+        })
+        const wrapper = mount(RouterViewMock, {
+          global: {
+            plugins: [
+              [
+                DataLoaderPlugin,
+                {
+                  router,
+                },
+              ],
+              router,
+            ],
+          },
+        })
+
+        l1.spy.mockRejectedValue(new NavigationResult('/?redirected=true'))
+
+        await router.push('/fetch')
+        await vi.advanceTimersByTimeAsync(1)
+        expect(router.currentRoute.value.fullPath).toBe('/fetch')
+        // no data
+        expect(wrapper.text()).toBe('false')
+
+        expect('NavigationResult').toHaveBeenWarned()
+        expect('key: "my-key"').toHaveBeenWarned()
+      })
+    })
   }
 )

src/data-loaders/defineLoader.ts

@@ -23,6 +23,7 @@ import {
   getCurrentContext,
   setCurrentContext,
   IS_SSR_KEY,
+  LOADER_SET_KEY,
 } from 'unplugin-vue-router/data-loaders'
 
 import { shallowRef } from 'vue'
@@ -31,6 +32,7 @@ import {
   toLazyValue,
 } from './createDataLoader'
 import type { ErrorDefault } from './types-config'
+import { warn } from '../core/utils'
 
 /**
  * Creates a data loader composable that can be exported by pages to attach the data loading to a route. In this version `data` is always defined.
@@ -215,6 +217,10 @@ export function defineBasicLoader<Data>(
           // let the navigation guard collect the result
           if (d instanceof NavigationResult) {
             to.meta[NAVIGATION_RESULTS_KEY]!.push(d)
+            // help users find non-exposed loaders during development
+            if (process.env.NODE_ENV !== 'production') {
+              warnNonExposedLoader({ to, options, useDataLoader })
+            }
           } else {
             entry.staged = d
             entry.stagedError = null
@@ -229,6 +235,12 @@ export function defineBasicLoader<Data>(
         //   e
         // )
         if (entry.pendingLoad === currentLoad) {
+          // help users find non-exposed loaders during development
+          if (process.env.NODE_ENV !== 'production') {
+            if (e instanceof NavigationResult) {
+              warnNonExposedLoader({ to, options, useDataLoader })
+            }
+          }
           // in this case, commit will never be called so we should just drop the error
           // console.log(`🚨 error in "${options.key}"`, e)
           entry.stagedError = e
@@ -412,6 +424,32 @@ export function defineBasicLoader<Data>(
   return useDataLoader
 }
 
+/**
+ * Dev only warning for loaders that return/throw NavigationResult but are not exposed
+ *
+ * @param to - [TODO:description]
+ * @param options - [TODO:description]
+ * @param useDataLoader - [TODO:description]
+ */
+function warnNonExposedLoader({
+  to,
+  options,
+  useDataLoader,
+}: {
+  to: RouteLocationNormalizedLoaded
+  options: DefineDataLoaderOptions_LaxData
+  useDataLoader: UseDataLoader
+}) {
+  const loaders = to.meta[LOADER_SET_KEY]
+  console.log(options.key)
+  if (loaders && !loaders.has(useDataLoader)) {
+    warn(
+      'A loader returned a NavigationResult but is not registered on the route. Did you forget to "export" it from the page component?' +
+        (options.key ? ` (loader key: "${options.key}")` : '')
+    )
+  }
+}
+
 export interface DefineDataLoaderOptions_LaxData
   extends DefineDataLoaderOptionsBase_LaxData {
   /**