-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(vue): Apply stateTransformer to attachments in Pinia Plugin #16034
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.
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
Conversation
packages/vue/src/pinia.ts
Outdated
@@ -28,14 +28,20 @@ export const createSentryPiniaPlugin: (options?: SentryPiniaPluginOptions) => Pi | |||
}, | |||
) => { | |||
const plugin: PiniaPlugin = ({ store, pinia }) => { | |||
const getAllStoreStates = (): Record<string, unknown> => { | |||
const getAllStoreStates = ( | |||
stateTransformer?: SentryPiniaPluginOptions['stateTransformer'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m: The default options need to be destructured into the options object so that people can provide only a few of the options but still get the rest of the defaults.
// declare the type with no optional fields
type SentryPiniaPluginOptions = {
attachPiniaState: boolean;
addBreadcrumbs: boolean;
actionTransformer: (action: string) => any;
stateTransformer: (state: Record<string, unknown>) => any;
};
const DEFAULT_PINIA_PLUGIN_OPTIONS: SentryPiniaPluginOptions = {
attachPiniaState: true,
addBreadcrumbs: true,
actionTransformer: action => action,
stateTransformer: state => state,
}
// make sure user options are `Partial<SentryPiniaPluginOptions>` so that they can define
// a subset of options they want to override.
export const createSentryPiniaPlugin: = (userOptions: Partial<SentryPiniaPluginOptions> = {}): PiniaPlugin => {
const { attachPiniaState, addBreadcrumbs, actionTransformer, stateTransformer } = { ...userOptions, ...DEFAULT_PINIA_PLUGIN_OPTIONS } as SentryPiniaPluginOptions;
This also means we don't need to pass in stateTransformer
into getAllStoreStates
, because it can grab it from the closure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that stateTransformer
could be grabbed from the closure here but I made it an argument of the function so it's more explicit that a stateTransformer
is used when calling getAllStates
. Otherwise it would be a weird side effect you only see when looking at the function code.
But good catch with the destructuring - right now it's either the users' object or the default one.
Continuation of #14474
As the store logic changed a bit, I changed the PR a bit. The
getAllStores
function can now receive thestateTransformer
and apply it.Closes #14441