feat(warn): detect global context on the server side (#2983)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Eduardo San Martin Morote <posva13@gmail.com>
Co-authored-by: Eduardo San Martin Morote <posva@users.noreply.github.com>

diff --git a/packages/pinia/__tests__/ssr.spec.ts b/packages/pinia/__tests__/ssr.spec.ts
index f9c80778fd85e6bfbf3d0202fc8ea4f5549b1eb1..a454fa9d474a1a0aed6d9060857d39a988bdadb3 100644 (file)
--- a/packages/pinia/__tests__/ssr.spec.ts
+++ b/packages/pinia/__tests__/ssr.spec.ts
@@ -2,13 +2,23 @@
  * @vitest-environment node
  */
 import { describe, it, expect } from 'vitest'
-import { createPinia, defineStore, shouldHydrate } from '../src'
+import {
+  createPinia,
+  defineStore,
+  getActivePinia,
+  setActivePinia,
+  shouldHydrate,
+} from '../src'
 import { Component, createSSRApp, inject, ref, computed, customRef } from 'vue'
 import { renderToString, ssrInterpolate } from '@vue/server-renderer'
 import { useUserStore } from './pinia/stores/user'
 import { useCartStore } from './pinia/stores/cart'
+import { mockConsoleError, mockWarn } from './vitest-mock-warn'
 
 describe('SSR', () => {
+  mockWarn()
+  mockConsoleError()
+
   const App = {
     ssrRender(ctx: any, push: any, _parent: any) {
       push(
@@ -162,6 +172,13 @@ describe('SSR', () => {
     }).not.toThrow()
   })
 
+  it('errors if getActivePinia called outside of context', async () => {
+    const pinia = createPinia()
+    setActivePinia(pinia)
+    expect(getActivePinia()).toBe(pinia)
+    expect('Pinia instance not found in context').toHaveBeenErrored()
+  })
+
   describe('Setup Store', () => {
     const useStore = defineStore('main', () => {
       const count = ref(0)

diff --git a/packages/pinia/__tests__/vitest-mock-warn.ts b/packages/pinia/__tests__/vitest-mock-warn.ts
index 0a374a154711cd001a2ac0634e172bb10d9fadcd..84292f12f83465173e3e100b7a20da6a6c851645 100644 (file)
--- a/packages/pinia/__tests__/vitest-mock-warn.ts
+++ b/packages/pinia/__tests__/vitest-mock-warn.ts
@@ -1,5 +1,4 @@
 // https://github.com/posva/jest-mock-warn/blob/master/src/index.js
-
 import type { MockInstance } from 'vitest'
 import { afterEach, beforeEach, expect, vi } from 'vitest'
 
@@ -7,6 +6,9 @@ interface CustomMatchers<R = unknown> {
   toHaveBeenWarned: () => R
   toHaveBeenWarnedLast: () => R
   toHaveBeenWarnedTimes: (n: number) => R
+  toHaveBeenErrored: () => R
+  toHaveBeenErroredLast: () => R
+  toHaveBeenErroredTimes: (n: number) => R
 }
 
 declare module 'vitest' {
@@ -14,118 +16,114 @@ declare module 'vitest' {
   interface AsymmetricMatchersContaining extends CustomMatchers {}
 }
 
-export function mockWarn() {
-  let warn: MockInstance<(typeof console)['log']>
+function createMockConsoleMethod(method: 'warn' | 'error') {
+  let mockInstance: MockInstance<(typeof console)[typeof method]>
   const asserted = new Map<string, string | RegExp>()
 
   expect.extend({
-    toHaveBeenWarned(received: string | RegExp) {
+    [`toHaveBeen${method.charAt(0).toUpperCase() + method.slice(1)}ed`](
+      received: string | RegExp
+    ) {
       asserted.set(received.toString(), received)
-      const passed = warn.mock.calls.some((args) =>
+      const passed = mockInstance.mock.calls.some((args) =>
         typeof received === 'string'
-          ? args[0].includes(received)
-          : received.test(args[0])
+          ? String(args[0]).includes(received)
+          : received.test(String(args[0]))
       )
-      if (passed) {
-        return {
-          pass: true,
-          message: () => `expected "${received}" not to have been warned.`,
-        }
-      } else {
-        const msgs = warn.mock.calls.map((args) => args[0]).join('\n - ')
-        return {
-          pass: false,
-          message: () =>
-            `expected "${received}" to have been warned.\n\nActual messages:\n\n - ${msgs}`,
-        }
-      }
+
+      return passed
+        ? {
+            pass: true,
+            message: () =>
+              `expected "${received}" not to have been ${method}ed.`,
+          }
+        : {
+            pass: false,
+            message: () => `expected "${received}" to have been ${method}ed.`,
+          }
     },
 
-    toHaveBeenWarnedLast(received: string | RegExp) {
+    [`toHaveBeen${method.charAt(0).toUpperCase() + method.slice(1)}edLast`](
+      received: string | RegExp
+    ) {
       asserted.set(received.toString(), received)
-      const lastCall = warn.mock.calls[warn.mock.calls.length - 1][0]
+      const lastCall = String(mockInstance.mock.calls.at(-1)?.[0])
       const passed =
         typeof received === 'string'
-          ? lastCall.includes(received)
+          ? lastCall?.includes(received)
           : received.test(lastCall)
-      if (passed) {
-        return {
-          pass: true,
-          message: () => `expected "${received}" not to have been warned last.`,
-        }
-      } else {
-        const msgs = warn.mock.calls.map((args) => args[0]).join('\n - ')
-        return {
-          pass: false,
-          message: () =>
-            `expected "${received}" to have been warned last.\n\nActual messages:\n\n - ${msgs}`,
-        }
-      }
+
+      return passed
+        ? {
+            pass: true,
+            message: () =>
+              `expected "${received}" not to have been ${method}ed last.`,
+          }
+        : {
+            pass: false,
+            message: () =>
+              `expected "${received}" to have been ${method}ed last.`,
+          }
     },
 
-    toHaveBeenWarnedTimes(received: string | RegExp, n: number) {
+    [`toHaveBeen${method.charAt(0).toUpperCase() + method.slice(1)}edTimes`](
+      received: string | RegExp,
+      n: number
+    ) {
       asserted.set(received.toString(), received)
-      let found = 0
-      warn.mock.calls.forEach((args) => {
-        const isFound =
-          typeof received === 'string'
-            ? args[0].includes(received)
-            : received.test(args[0])
-        if (isFound) {
-          found++
-        }
-      })
+      const count = mockInstance.mock.calls.filter((args) =>
+        typeof received === 'string'
+          ? String(args[0]).includes(received)
+          : received.test(String(args[0]))
+      ).length
 
-      if (found === n) {
-        return {
-          pass: true,
-          message: () =>
-            `expected "${received}" to have been warned ${n} times.`,
-        }
-      } else {
-        return {
-          pass: false,
-          message: () =>
-            `expected "${received}" to have been warned ${n} times but got ${found}.`,
-        }
-      }
+      return count === n
+        ? {
+            pass: true,
+            message: () =>
+              `expected "${received}" to have been ${method}ed ${n} times.`,
+          }
+        : {
+            pass: false,
+            message: () =>
+              `expected "${received}" to have been ${method}ed ${n} times but got ${count}.`,
+          }
     },
   })
 
   beforeEach(() => {
     asserted.clear()
-    warn = vi.spyOn(console, 'warn')
-    warn.mockImplementation(() => {})
+    mockInstance = vi.spyOn(console, method).mockImplementation(() => {})
   })
 
   afterEach(() => {
     const assertedArray = Array.from(asserted)
-    const nonAssertedWarnings = warn.mock.calls
-      .map((args) => args[0])
-      .filter((received) => {
-        return !assertedArray.some(([_key, assertedMsg]) => {
-          return typeof assertedMsg === 'string'
-            ? received.includes(assertedMsg)
-            : assertedMsg.test(received)
-        })
-      })
-    warn.mockRestore()
-    if (nonAssertedWarnings.length) {
-      nonAssertedWarnings.forEach((warning) => {
-        console.warn(warning)
+    const unassertedLogs = mockInstance.mock.calls
+      .map((args) => String(args[0]))
+      .filter(
+        (msg) =>
+          !assertedArray.some(([_key, assertedMsg]) =>
+            typeof assertedMsg === 'string'
+              ? msg.includes(assertedMsg)
+              : assertedMsg.test(msg)
+          )
+      )
+
+    mockInstance.mockRestore()
+
+    if (unassertedLogs.length) {
+      unassertedLogs.forEach((msg) => console[method](msg))
+      throw new Error(`Test case threw unexpected ${method}s.`, {
+        cause: unassertedLogs,
       })
-      throw new Error(`test case threw unexpected warnings.`)
     }
   })
 }
 
-interface CustomMatchers<R = unknown> {
-  toHaveBeenWarned: () => R
-  toHaveBeenWarnedLast: () => R
-  toHaveBeenWarnedTimes: (n: number) => R
+export function mockWarn() {
+  createMockConsoleMethod('warn')
 }
 
-declare module 'vitest' {
-  interface Assertion<T = any> extends CustomMatchers<T> {}
-  interface AsymmetricMatchersContaining extends CustomMatchers {}
+export function mockConsoleError() {
+  createMockConsoleMethod('error')
 }

diff --git a/packages/pinia/src/rootStore.ts b/packages/pinia/src/rootStore.ts
index 2de789f34dee7ab84a53b24c1b5433ddcfc87e5c..f69544c3a8b4661776b3d5a32c40a8c9f6cd0652 100644 (file)
--- a/packages/pinia/src/rootStore.ts
+++ b/packages/pinia/src/rootStore.ts
@@ -17,6 +17,7 @@ import {
   DefineStoreOptionsInPlugin,
   StoreGeneric,
 } from './types'
+import { IS_CLIENT } from './env'
 
 /**
  * setActivePinia must be called to handle SSR at the top of functions like
@@ -39,11 +40,29 @@ interface _SetActivePinia {
   (pinia: Pinia | undefined): Pinia | undefined
 }
 
+declare global {
+  interface ImportMeta {
+    server?: boolean
+  }
+}
 /**
  * Get the currently active pinia if there is any.
  */
-export const getActivePinia = () =>
-  (hasInjectionContext() && inject(piniaSymbol)) || activePinia
+export const getActivePinia = __DEV__
+  ? (): Pinia | undefined => {
+      const pinia = hasInjectionContext() && inject(piniaSymbol)
+
+      if (!pinia && !IS_CLIENT) {
+        console.error(
+          `[🍍]: Pinia instance not found in context. This falls back to the global activePinia which exposes you to cross-request pollution on the server. Most of the time, it means you are calling "useStore()" in the wrong place.\n` +
+            `Read https://vuejs.org/guide/reusability/composables.html to learn more`
+        )
+      }
+
+      return pinia || activePinia
+    }
+  : (): Pinia | undefined =>
+      (hasInjectionContext() && inject(piniaSymbol)) || activePinia
 
 /**
  * Every application must own its own pinia to be able to create stores