Skip to content

Commit be9e356

Browse files
ivanskycoderabbitai[bot]posva
authored
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>
1 parent 8a65eb7 commit be9e356

File tree

3 files changed

+120
-86
lines changed

3 files changed

+120
-86
lines changed

‎packages/pinia/__tests__/ssr.spec.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,23 @@
22
* @vitest-environment node
33
*/
44
import { describe, it, expect } from 'vitest'
5-
import { createPinia, defineStore, shouldHydrate } from '../src'
5+
import {
6+
createPinia,
7+
defineStore,
8+
getActivePinia,
9+
setActivePinia,
10+
shouldHydrate,
11+
} from '../src'
612
import { Component, createSSRApp, inject, ref, computed, customRef } from 'vue'
713
import { renderToString, ssrInterpolate } from '@vue/server-renderer'
814
import { useUserStore } from './pinia/stores/user'
915
import { useCartStore } from './pinia/stores/cart'
16+
import { mockConsoleError, mockWarn } from './vitest-mock-warn'
1017

1118
describe('SSR', () => {
19+
mockWarn()
20+
mockConsoleError()
21+
1222
const App = {
1323
ssrRender(ctx: any, push: any, _parent: any) {
1424
push(
@@ -162,6 +172,13 @@ describe('SSR', () => {
162172
}).not.toThrow()
163173
})
164174

175+
it('errors if getActivePinia called outside of context', async () => {
176+
const pinia = createPinia()
177+
setActivePinia(pinia)
178+
expect(getActivePinia()).toBe(pinia)
179+
expect('Pinia instance not found in context').toHaveBeenErrored()
180+
})
181+
165182
describe('Setup Store', () => {
166183
const useStore = defineStore('main', () => {
167184
const count = ref(0)

‎packages/pinia/__tests__/vitest-mock-warn.ts‎

Lines changed: 81 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,131 +1,129 @@
11
// https://github.com/posva/jest-mock-warn/blob/master/src/index.js
2-
32
import type { MockInstance } from 'vitest'
43
import { afterEach, beforeEach, expect, vi } from 'vitest'
54

65
interface CustomMatchers<R = unknown> {
76
toHaveBeenWarned: () => R
87
toHaveBeenWarnedLast: () => R
98
toHaveBeenWarnedTimes: (n: number) => R
9+
toHaveBeenErrored: () => R
10+
toHaveBeenErroredLast: () => R
11+
toHaveBeenErroredTimes: (n: number) => R
1012
}
1113

1214
declare module 'vitest' {
1315
interface Assertion<T = any> extends CustomMatchers<T> {}
1416
interface AsymmetricMatchersContaining extends CustomMatchers {}
1517
}
1618

17-
export function mockWarn() {
18-
let warn: MockInstance<(typeof console)['log']>
19+
function createMockConsoleMethod(method: 'warn' | 'error') {
20+
let mockInstance: MockInstance<(typeof console)[typeof method]>
1921
const asserted = new Map<string, string | RegExp>()
2022

2123
expect.extend({
22-
toHaveBeenWarned(received: string | RegExp) {
24+
[`toHaveBeen${method.charAt(0).toUpperCase() + method.slice(1)}ed`](
25+
received: string | RegExp
26+
) {
2327
asserted.set(received.toString(), received)
24-
const passed = warn.mock.calls.some((args) =>
28+
const passed = mockInstance.mock.calls.some((args) =>
2529
typeof received === 'string'
26-
? args[0].includes(received)
27-
: received.test(args[0])
30+
? String(args[0]).includes(received)
31+
: received.test(String(args[0]))
2832
)
29-
if (passed) {
30-
return {
31-
pass: true,
32-
message: () => `expected "${received}" not to have been warned.`,
33-
}
34-
} else {
35-
const msgs = warn.mock.calls.map((args) => args[0]).join('\n - ')
36-
return {
37-
pass: false,
38-
message: () =>
39-
`expected "${received}" to have been warned.\n\nActual messages:\n\n - ${msgs}`,
40-
}
41-
}
33+
34+
return passed
35+
? {
36+
pass: true,
37+
message: () =>
38+
`expected "${received}" not to have been ${method}ed.`,
39+
}
40+
: {
41+
pass: false,
42+
message: () => `expected "${received}" to have been ${method}ed.`,
43+
}
4244
},
4345

44-
toHaveBeenWarnedLast(received: string | RegExp) {
46+
[`toHaveBeen${method.charAt(0).toUpperCase() + method.slice(1)}edLast`](
47+
received: string | RegExp
48+
) {
4549
asserted.set(received.toString(), received)
46-
const lastCall = warn.mock.calls[warn.mock.calls.length - 1][0]
50+
const lastCall = String(mockInstance.mock.calls.at(-1)?.[0])
4751
const passed =
4852
typeof received === 'string'
49-
? lastCall.includes(received)
53+
? lastCall?.includes(received)
5054
: received.test(lastCall)
51-
if (passed) {
52-
return {
53-
pass: true,
54-
message: () => `expected "${received}" not to have been warned last.`,
55-
}
56-
} else {
57-
const msgs = warn.mock.calls.map((args) => args[0]).join('\n - ')
58-
return {
59-
pass: false,
60-
message: () =>
61-
`expected "${received}" to have been warned last.\n\nActual messages:\n\n - ${msgs}`,
62-
}
63-
}
55+
56+
return passed
57+
? {
58+
pass: true,
59+
message: () =>
60+
`expected "${received}" not to have been ${method}ed last.`,
61+
}
62+
: {
63+
pass: false,
64+
message: () =>
65+
`expected "${received}" to have been ${method}ed last.`,
66+
}
6467
},
6568

66-
toHaveBeenWarnedTimes(received: string | RegExp, n: number) {
69+
[`toHaveBeen${method.charAt(0).toUpperCase() + method.slice(1)}edTimes`](
70+
received: string | RegExp,
71+
n: number
72+
) {
6773
asserted.set(received.toString(), received)
68-
let found = 0
69-
warn.mock.calls.forEach((args) => {
70-
const isFound =
71-
typeof received === 'string'
72-
? args[0].includes(received)
73-
: received.test(args[0])
74-
if (isFound) {
75-
found++
76-
}
77-
})
74+
const count = mockInstance.mock.calls.filter((args) =>
75+
typeof received === 'string'
76+
? String(args[0]).includes(received)
77+
: received.test(String(args[0]))
78+
).length
7879

79-
if (found === n) {
80-
return {
81-
pass: true,
82-
message: () =>
83-
`expected "${received}" to have been warned ${n} times.`,
84-
}
85-
} else {
86-
return {
87-
pass: false,
88-
message: () =>
89-
`expected "${received}" to have been warned ${n} times but got ${found}.`,
90-
}
91-
}
80+
return count === n
81+
? {
82+
pass: true,
83+
message: () =>
84+
`expected "${received}" to have been ${method}ed ${n} times.`,
85+
}
86+
: {
87+
pass: false,
88+
message: () =>
89+
`expected "${received}" to have been ${method}ed ${n} times but got ${count}.`,
90+
}
9291
},
9392
})
9493

9594
beforeEach(() => {
9695
asserted.clear()
97-
warn = vi.spyOn(console, 'warn')
98-
warn.mockImplementation(() => {})
96+
mockInstance = vi.spyOn(console, method).mockImplementation(() => {})
9997
})
10098

10199
afterEach(() => {
102100
const assertedArray = Array.from(asserted)
103-
const nonAssertedWarnings = warn.mock.calls
104-
.map((args) => args[0])
105-
.filter((received) => {
106-
return !assertedArray.some(([_key, assertedMsg]) => {
107-
return typeof assertedMsg === 'string'
108-
? received.includes(assertedMsg)
109-
: assertedMsg.test(received)
110-
})
111-
})
112-
warn.mockRestore()
113-
if (nonAssertedWarnings.length) {
114-
nonAssertedWarnings.forEach((warning) => {
115-
console.warn(warning)
101+
const unassertedLogs = mockInstance.mock.calls
102+
.map((args) => String(args[0]))
103+
.filter(
104+
(msg) =>
105+
!assertedArray.some(([_key, assertedMsg]) =>
106+
typeof assertedMsg === 'string'
107+
? msg.includes(assertedMsg)
108+
: assertedMsg.test(msg)
109+
)
110+
)
111+
112+
mockInstance.mockRestore()
113+
114+
if (unassertedLogs.length) {
115+
unassertedLogs.forEach((msg) => console[method](msg))
116+
throw new Error(`Test case threw unexpected ${method}s.`, {
117+
cause: unassertedLogs,
116118
})
117-
throw new Error(`test case threw unexpected warnings.`)
118119
}
119120
})
120121
}
121122

122-
interface CustomMatchers<R = unknown> {
123-
toHaveBeenWarned: () => R
124-
toHaveBeenWarnedLast: () => R
125-
toHaveBeenWarnedTimes: (n: number) => R
123+
export function mockWarn() {
124+
createMockConsoleMethod('warn')
126125
}
127126

128-
declare module 'vitest' {
129-
interface Assertion<T = any> extends CustomMatchers<T> {}
130-
interface AsymmetricMatchersContaining extends CustomMatchers {}
127+
export function mockConsoleError() {
128+
createMockConsoleMethod('error')
131129
}

‎packages/pinia/src/rootStore.ts‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
DefineStoreOptionsInPlugin,
1818
StoreGeneric,
1919
} from './types'
20+
import { IS_CLIENT } from './env'
2021

2122
/**
2223
* setActivePinia must be called to handle SSR at the top of functions like
@@ -39,11 +40,29 @@ interface _SetActivePinia {
3940
(pinia: Pinia | undefined): Pinia | undefined
4041
}
4142

43+
declare global {
44+
interface ImportMeta {
45+
server?: boolean
46+
}
47+
}
4248
/**
4349
* Get the currently active pinia if there is any.
4450
*/
45-
export const getActivePinia = () =>
46-
(hasInjectionContext() && inject(piniaSymbol)) || activePinia
51+
export const getActivePinia = __DEV__
52+
? (): Pinia | undefined => {
53+
const pinia = hasInjectionContext() && inject(piniaSymbol)
54+
55+
if (!pinia && !IS_CLIENT) {
56+
console.error(
57+
`[🍍]: 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` +
58+
`Read https://vuejs.org/guide/reusability/composables.html to learn more`
59+
)
60+
}
61+
62+
return pinia || activePinia
63+
}
64+
: (): Pinia | undefined =>
65+
(hasInjectionContext() && inject(piniaSymbol)) || activePinia
4766

4867
/**
4968
* Every application must own its own pinia to be able to create stores

0 commit comments

Comments
 (0)