Figma helpers seem too opinionated to be applied to different types of plugins

[gusso]

onmessage

As far as I understand how onmessage works, adding multiple ones across a project will make it unpredictable which one will receive the message. Considering most plugins will already use another onmessage, adding one inside a helper method can cause it to start intercepting messages from the rest of the plugin. I'd consider making this a code snippet so people see how they can set it up, and having the helper not deal with receiving the message itself.

https://github.com/rdjpalmer/no-persistence-mixpanel/blob/57b7488a44ab3585db947bc3145cd700fce9f79f/src/figma-ui.ts#L9-L16

Here is an example on how I set up my global onmessage for a plugin, which then acts on whatever the message is:

onmessage = ({ data }) => {
  const message = data.pluginMessage

  switch (message.name) {
    case "getCachedScreens":
      setScreens(message.screens)
      break

    case "mixpanel-identification":
      mixpanel.identify(message.userId)
      mixpanel.track("Plugin Opened")
      break

    case "showUI":
      setIsVisible(true)
      break

    default:
      break
  }
}

Message ID

If you agree with that, this means that posting the message can also be problematic, because you're being opinionated about the naming of the message object. For example, you are referencing the message ID as type, while I call it name on my switch statement (above). This means this helper also doesn't work for my project, unless I rename a whole bunch of stuff in my project to accommodate to the type property.

https://github.com/rdjpalmer/no-persistence-mixpanel/blob/57b7488a44ab3585db947bc3145cd700fce9f79f/src/figma-code.ts#L15-L18

How the package is currently useful for me

The real value of the package is in the core Mixpanel wrapper of course, and it's still a giant lifesaver and I'll use it all the time, but the plugin helpers confused me because they didn't work for my setup. That made me think for a moment that the whole thing wouldn't work for me. It would be a shame if people didn't realise that, for many plugin setups, the helpers are overkill and could do more damage than good.

[rdjpalmer]

There's two different takes on this, and improvements to come from both:

1. Documentation should be updated to be used as you described (i.e., force the consumer to do the plumbing, listening of message, passing the values to mixpanel etc.)
2. The helper method shouldn't require a second onmessage function

The former is definitely straight forward and I'll add that to the docs.

The latter, I think the issue is actually that it doesn't pass on the message to the onInitialised function:

https://github.com/rdjpalmer/no-persistence-mixpanel/blob/57b7488a44ab3585db947bc3145cd700fce9f79f/src/figma-ui.ts#L9-L16

Should be something like:

window.onmessage = (event) => {
  const message = event.data.pluginMessage;

  if (message.type === "mixpanel-identification") {
    mixpanel.identify(message.userId);
    onInitialised(null, mixpanel);
  } else {
    onInitialised(event, mixpanel);
  }
};

Which would then allow you to replace your onmessage listener without fault:

init(/* token */, (event, mixpanel) => {
  const message = data.pluginMessage

  switch (message.name) {
    case "getCachedScreens":
      setScreens(message.screens)
      break

    case "showUI":
      setIsVisible(true)
      break

    default:
      break
  }
});

The case for the helper method is to allow for the library to do more for the user. It can ensure that you've first initialised mixpanel, and it could also be developed to ensure that before initialisation that events can be tracked in memory and flushed after initialisation.

I wonder if we could also use addEventListener instead of onmessage to remove the need for wrapping at all, and allow setup to be as simple as it would be now? Docs suggest no.

With regards to the message's structure, that's quite the conundrum! The format I've used takes inspiration from Redux, where the actions have type and payload properties.

Beyond the scope this codebase, but it might be worth providing a helper lib to ensure consistency across messages? They could quite easily become:

figma.ui.postMessage(CreateMessage(Mixpanel.message.Initialize, { userId: figma.currentUser.id });
figma.ui.postMessage(CreateMessage("getCachedScreens", { screens });

If possible, it would be good to avoid the consumer having to plumb this in though, as it'll lead to a nightmare if the user ID is missing.

Let me know what you think about the above @gusso. Happy to change up the Figma specific code as you see fit!

[gusso]

Which would then allow you to replace your onmessage listener without fault:

Ha, this looks like it would work well, nice! I think it might be a little weird to not see the listener and rely on the analytics package to set it up, and keep that in mind by only reading the readme. The fact that a function named init pipes in every message from the plugin code feels a little magical. What if the name of init described that also? Something along the lines of initAndListen? But better 😂

With regards to the message's structure, that's quite the conundrum!

Yea I know, I don't like that they don't suggest anything on the official docs. But for this reason I think it might be unreasonable to force people to use a specific structure because of this package. I definitely have barely any opinion about what term is best to identify the message, it really is not about me disagreeing with type at all. But it just doesn't feel like adapting to this should be a required step to use the package.

[rdjpalmer]

Great points RE naming. For devs it should be a neat enough abstraction though, similar to how NextJS's plugins work (e.g., withSourceMaps). This is probably a good naming convention too. And can solve a little of the magic by exposing onmessage like usual:

window.onmessage = withMixpanel((event, mixpanel) => {...}, mixpanelOptions);

By being able to handle all events, we can actively avoid the need to handle the initialisation event too, so the internal handler should be able to work without caring for the structure of lib's internal event.

How does this sound @gusso?

[gusso]

Ohhh!? This sounds absolutely perfect! Will love to review a PR with this 👌