mixpanel / mixpanel-js

Official Mixpanel JavaScript Client Library
https://mixpanel.com/help/reference/javascript
Other
862 stars 308 forks source link

Support for Figma plugins or other Iframe enviroments #388

Open andirsun opened 8 months ago

andirsun commented 8 months ago

Problem

Mixpanel client will not work in Figma plugins UI.

Why?

Electron limitation when Iframe content is loaded as Data-URI. Both localStorage and document.cookie are not available (similar problem).

Mixpanel client has configuration option to switch off persistance disable_persistence, but it won't help, as it accesses cookie, which crashes the client

tdumitrescu commented 8 months ago

We're looking at providing a simpler "out of the box" config option that will make the SDK not touch cookies or localStorage at all (means you have to provide your own user IDs if you want to link any activity across pages). Hopefully that would address this.

If you can provide a stack trace or something showing exactly where it crashes when disable_persistence is on, that would be useful (probably a GDPR opt-in/out check, but good to know anyway)

andirsun commented 8 months ago

I am going to provide the stack trace to solve this, thanks!

andirsun commented 8 months ago

@tdumitrescu this is my code:

mixpanelFigma.init(config.MIXPANEL_PROJECT_TOKEN, {
   disable_persistence: true,
})

This is the error:

SecurityError: Failed to set the 'cookie' property on 'Document': Cookies are disabled inside 'data:' URLs.
tdumitrescu commented 8 months ago

Thanks @andirsun . You don't have any trace of where this error occurs in the code?

andirsun commented 8 months ago

I was investigating and the only way to access localStorage in figma plugins is with this API

You don't have any trace of where this error occurs in the code?

You mean my code or the figma code ? I can not initialize the library.

tdumitrescu commented 8 months ago

I mean that somewhere in the library code, a JS statement is trying to execute, and when that happens the exception SecurityError: Failed to set the 'cookie' property on 'Document' is getting thrown. It would be useful to know which statement it's trying to execute when that happens, which you can usually get from the JS console in the browser.

andirsun commented 8 months ago

@tdumitrescu Is something around these lines

https://github.com/mixpanel/mixpanel-js/blob/1c4d98b4a485fbf4dc4421f00c33f3b19530b307/dist/mixpanel.cjs.js#L1038

I think this lines dont have to check for document$1.cookie if I set disable_persistence = false on the library init config

And for example here https://github.com/mixpanel/mixpanel-js/blob/1c4d98b4a485fbf4dc4421f00c33f3b19530b307/dist/mixpanel.cjs.js#L1099

We can check with something like

if (figma && figma.clientStorage) {
  figma.clientStorage.setAsync(name, value)
} else {
  window.localStorage.setItem(name, value); 
}

The goal is to assume document.localStorage and document.cookie are not available until we validate them In case figma is detected we can use the native api to access local storage

Screenshot 2023-09-21 at 23 32 05
tdumitrescu commented 8 months ago

Thanks. If you can provide any more of the stack leading up to those lines, that would be useful, as those are just the innermost setters. Anyway, no big deal if you can't, we'll need to set up a similar environment to test any fixes anyway.

As for the figma clientStorage alternative, unfortunately it can't be used as a drop-in replacement for cookie/localstorage persistence, as it appears to offer only an asynchronous API (https://www.figma.com/plugin-docs/api/figma-clientStorage/). The assumption that persisted values can be stored and fetched synchronously is pretty deeply ingrained in this lib's code. But we should be able to fix the lib pretty easily so that it can operate without touching cookies/localstorage at all. Then you'd need to manage your own persistence (storing any superproperties you want, such as distinct ID).

andirsun commented 8 months ago

Thanks for looking into this @tdumitrescu ! A couple of questions:

tdumitrescu commented 8 months ago

The way others have done it in the past is to set a distinct ID superproperty manually, rather than going through the identify() call, though that's suboptimal. It would take a small lib update not to auto-generate a random distinct ID automatically, but after that fix it would look like the normal identification flow:

mixpanel.init(`<TOKEN>`, {disable_persistence: true});
mixpanel.identify(`<your user id>`);

As long as the identify call directly follows the init call, then your ID should be used for all tracking calls.

andirsun commented 8 months ago

@tdumitrescu Sounds great!, let me know if you need help with the patch test, I can use our plugin to test or provide you a starter repo.

andirsun commented 8 months ago

Hey! @tdumitrescu do you maybe have some progress with this ?

andirsun commented 3 months ago

Ping

doc-han commented 1 month ago

@andirsun this is quite the same issue the browser extension community is facing using mixpanel-js. The library implementation is synchronous and also some apis used from the navigator API might not be available in other environments. Hence, you mostly just have to write your own code. example implementation for browser extensions You can edit that.

andirsun commented 1 month ago

@doc-han Thanks for the detailed response. I think we can implement that solution you recommend.

But do you think you can modify the mixpanel-js library to avoid using cookies and localstorage when passing disable_persistence=false https://github.com/mixpanel/mixpanel-js/issues/388#issuecomment-1730329070

This is something mixpanel could consider ? To be honest I would prefer to use the official library instead of using patched versions to support mixpanel on figma environment.