Skip to content

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

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

Open
wants to merge 3 commits into
base: v3
Choose a base branch
from
Open

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

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e1419d7
fix: never use global context on the server side
ivansky May 27, 2025
d33bd37
Improve error message
ivansky May 27, 2025
56d62d0
Update rootStore.ts
ivansky May 27, 2025
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
2 changes: 1 addition & 1 deletion packages/pinia/src/rootStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ interface _SetActivePinia {
* Get the currently active pinia if there is any.
*/
export const getActivePinia = () =>
(hasInjectionContext() && inject(piniaSymbol)) || activePinia
(hasInjectionContext() && inject(piniaSymbol)) || (import.meta.server ? throw new Error("Cannot get active pinia as it does not find context") : activePinia)
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the PR. This could be changed into a non breaking change by just showing an error and still returning the active pinia. The error should be in development only (like other warnings) and should not rely on import as it might not work in all scenarios

Copy link
Author

Choose a reason for hiding this comment

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

I think failing fast is the better approach when the global activePinia is accessed on the server, because:

  1. failing immediately makes the problem obvious rather than silently continuing with potentially incorrect behavior.
  2. allowing it to continue by returning activePinia could lead to dangerous state sharing between requests, which is particularly problematic in server environments.

maybe making the message is more clear would help huge

Copy link
Member

Choose a reason for hiding this comment

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

That would be a breaking change. Also, I do prefer the error message because it feels less frustrating to users

Copy link
Author

Choose a reason for hiding this comment

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

I will leave the decision to you,

However, please keep in mind that the developers tend to ignore error messages, so they will make the same mistakes again and again, I would prefer it to fail that will indicate the mistake much faster.


/**
* Every application must own its own pinia to be able to create stores
Expand Down