Clean up extensions approach

[Twixes]

Changes

Stemming from #247 (comment), this is a way to make extensions more usable.
Currently some of them are part of plugin meta, while others are globals. And the latter half is not usable in TypeScript plugins: it requires plugin developers to write their own definition of the extension and to convince TypeScript that the global will surely be injected at runtime. With the meta object on the other hand it's just a matter of using PluginMeta from the plugin scaffold and everything works.
This PR injects everything into both globals and meta, with a unified createExtensions in extensions/index, which allows the plugin server (VM generation in particular) to be agnostic of particular extensions.

Checklist

• Jest tests

[mariusandra]

For the record, I don't see anything inherently wrong with having some things in meta and some as global. The separation was really clear before: 1) cache, storage, and other things that are internal to the plugin server in meta, 2) all exported libraries (bigquery, fetch, etc) in globals.

[Twixes]

From my side, I just can't practically use injected globals with .ts plugins, which is frustrating. This is why I push for something like this.

[mariusandra]

I still don't fully get that. It should be enough to add into @posthog/plugin-scaffold/src/types.ts lines like the following:

declare const posthog: {
    capture: (event: string, properties?: Record<string, any>) => void
}

... and then in whatever source file that imports this package (or any other package where these types are in the file that's defined by the types field in package.json), we'll have all globals as we want them:

The problem is obviously that this package is imported not just by plugins, but also by posthog itself... and we probably don't want to add all plugin server globals in the frontend there. So some side-package (separate scaffold just for plugins) is probably needed.

[Twixes]

Yep. But… we'd need another package and we do actually want to share the definitions that are in plugin scaffold between the server and plugins to ensure we keep them in sync. This requires nothing additional. Just everything conveniently in meta as well as globally for backwards compatibility.

[mariusandra]

We can always make a new package, @posthog/plugin-globals, which would contain all types for all exported globals, pinned to the right versions. It's a better idea to do that in a the scaffold, as it'll need to add to its dependencies most of the google/maxmind/etc libraries just to import their types. Adding this to the main scaffold and thus the main posthog app is a bit overkill.

To make the experience seamless for users, this package would be included in the package.json of various plugin starter kits and pulled in via tsconfig.json, with something like this:

{
    "compilerOptions": {
        "target": "ES2018",
        "lib": ["ESNext"],
        "module": "ES2015",
        "moduleResolution": "Node",
        "strict": true,
        "forceConsistentCasingInFileNames": true,
        "noImplicitReturns": true,
        "noUnusedParameters": true,
        "noEmit": true,
        "resolveJsonModule": true
    },
    "exclude": ["**.test.ts"],
    "include": ["**.ts", "./node_modules/@posthog/plugin-globals/src/globals.d.ts"]
}

Still no? 😅

[mariusandra]

I've published this package that adds supports for plugin globals and is quite easy to install: https://github.com/PostHog/plugin-globals

[Twixes]

From a technical standpoint that is cool and does the job, but from an ergonomics point of view I really don't want to have to install an additional package and configure tsconfig for it (and as a PostHogger, I don't want to have to maintain it in addition to plugin-scaffold). I just want things to work with a single import line. Putting extensions into meta does just that.
Additionally I think meta is a great way to highlight that the extensions we offer are strictly distinct from a browser/Node environment, for instance that our meta.console object in reality does something very different from (logging into DB) from global.console you get in Node.

[mariusandra]

We can always use github actions to keep various packages in sync and thus have just one package ;).

I guess we need tiebreakers. @macobo ?

The question is basically:

function runEveryMinute() {
  posthog.capture('hello')
}

vs

import { Plugin } from '@posthog/posthog-scaffold'

const plugin: Plugin = {
  runEveryMinute(meta) {
    meta.posthog.capture('hello')
  }
}

module.exports = plugin

I'm still arguing that the former is a much nicer and more approachable API than the latter, even though the latter is more explicit... and what's more, I'm arguing that making it more approachable matters for the adoption of plugins and for the dev experience of people writing plugins (types are not everything).

Hence I still wouldn't put all external libraries into meta, but would keep them global. It makes writing code friendlier (just write fetch instead of meta.fetch) and the TS typings can be handled either by the @posthog/plugin-globals package or some github action that syncs this with the @posthog/plugin-scaffold package if we decide to go that route.

[Twixes]

Hm, that's not exactly the gist of the discussion (the Plugin type being a different one) and only a part of the boilerplate. In reality it's:

1. We have to maintain @posthog/plugin-scaffold AND @posthog/plugin-globals. Plugin implementation:

   // tsconfig.json
   "compilerOptions": {
       "types": ["../@posthog/plugin-globals"],
   }

   // package.json
   "devDependencies": {
       "@posthog/plugin-globals": "a.b.c",
       "@posthog/plugin-scaffold": "x.y.z",
   }

   // index.ts
   import { PluginMeta } from '@posthog/plugin-scaffold'
   
   function runEveryMinute(meta: PluginMeta) {
       posthog.capture('hello')
   }

2. Versus: we only have to maintain @posthog/plugin-scaffold. Plugin implementation:

   // package.json
   "devDependencies": {
       "@posthog/plugin-scaffold": "x.y.z",
   }

   // index.ts
   import { PluginMeta } from '@posthog/plugin-scaffold'
   
   function runEveryMinute(meta: PluginMeta) {
       meta.posthog.capture('hello')
   }

[Twixes]

This is the worst

[mariusandra]

This discussion is running off the rails if the arguments are in the format of "no because we need to maintain two packages".

Again, in my book this is a discussion between optics and practicality on one side and purity on the other. In my experience purity usually loses in the real world.

[Twixes]

But two packages are, in very practical terms: 1. more maintenance for us 2. more work to get going as a plugin server developer, especially if the new package also requires tsconfig setup.
@posthog/plugin-globals is pure and elegant in making use of TypeScript declaration features, but what I'm saying is precisely that it's not practical to me as someone who's writing plugins and as someone working on their backend.

[mariusandra]

To keep things simple, I'd actually just integrate this package into @posthog/plugin-scaffold. Apart from plugins themselves, the two other places that use this package (plugin-server and posthog) can easily work around it by adding one line to the excludes list in tsoconfig (or just actually not use any globals in the few files it'll be imported... though that may cascade down through other imports).

[mariusandra]

More practically, posthog and plugin-server could both import { ... } from '@posthog/plugin-scaffold/src/types.ts' without getting these globals and we could have an index.ts in the package that re-exports everything from globals.d.ts and types.ts, so that if you import { ... } from '@posthog/plugin-server', you'll get the globals automatically.

This solves all problems I see in the thread (you'll get globals in plugins without adding any extra lines anywhere) and is not that hard for people who code posthog to get their heads around. We could even just export * from '@posthog/plugin-scaffold/src/types.ts' inside types.ts in the projects and be none the wiser about any global magic.

[mariusandra]

There's one alternative approach we can take as well. Since we're using babel to transpile all plugin code, we could use actual import statements as well.

So to fetch you would type import * as fetch from 'node-fetch' and behind the scenes, babel could transpile that to const fetch = globalThis.fetch or something like that. The big problem will be that this will make it feel like users could import anything, but we'll actually only allow importing from some whitelisted virtual packages, and then also not everything. Thus I'm not sure if this adds a lot to the already proposed approaches.

And finally, I still like the logic that external libraries come to you externally and globally, not via any of the built in plugin objects. We are writing our own JS runtime here, so we can be really flexible with the syntax we offer our users.

[macobo]

I'm probably not the best person to judge in on this (least amount of plugins created). Instead, I'd ask:

• @yakkomajuri
• Plugin authors in users chat

However let's stir the pot a bit.

Let me try to summarize the argument(s):

1. We should decide where to put functions/classes exposed by extensions.
   • What extensions are is undocumented at the moment, but from the code is things from storage, exposing bigquery, exposing posthog-lite and console.log
2. The argument for putting most things to be global is practicality - it makes writing plugins easier
3. Using globals creates a headache around typing - users would need to modify tsconfig (and potentially include one more package)

My vote on the current state is:

• expose console as part of globals
• Everything else not as a global (including posthog)

Why? Because my argument is:
4. Users should be able to run and test the scripts they write outside of plugin server.

console.log is normally available with node.js runtimes and ergonomically it makes sense to have it at the fingertips.

However, as soon as your functions/scripts start relying on magical globals it becomes non-interoperable with the rest of the world.
With meta at least there's a clear way to pass in neccessary code from the outside to test and use in e.g. application code.

---

There were a few other ideas floated around like using babel to do some import-level schenanigans.

Mocking node-fetch this way would lead (imo) to a lot of confusion,
but we technically could provide packages like @posthog-plugins/fetch, @posthog-plugins/bigquery and so on.
Packages from that namespace would be "available" for plugins via this babel schenanigans. However it could still cause trouble for users
when they stay on older posthog versions or there's other versioning-related troubles. That's an easy way to cause days of debugging for someone
as things work locally but not in 'prod'.

[mariusandra]

Thanks @macobo for weighing in! This actually made me really like the approach of importing from @posthog-plugins/bigquery. I'll give my arguments below.

There's one thing that's slightly annoying with both previously discussed approaches (globals and meta), and that's that they make it a lot more complicated to write tests, for the plugin itself. Imagine you have an index.ts file that contains the plugin code, and either 1) gets google.bigquery from a global... and must somehow export it as a global to be able to run index.test.ts that imports index.ts and needs the global (mucking with jest config anyone?), or 2) manually import it and inject it into meta when calling the relevant functions. Here option 2 (meta) sounds better, but it's still quite sketchy.

If we instead re-package each plugin into something like @posthog-plugins/bigquery, we could actually yarn add and import from this same package in both the independent test setup, and inside the plugin server, where we would do some babel magic to link it to the right place.

Packaging things like this also gives us much greater control over versioning. In case the @google-cloud/bigquery plugin changes (could be often), we will anyway almost never update it in the plugin-server package.json. You could then import it in your tests and actually use the wrong version. If we publish our own packages for all extensions, the versions would be strictly pinned... and if any of our own packages has a new version, plugin authors can be sure to know that's the one which is used in the plugin server.

Re-exporting the package as @posthog-plugins/bigquery should not require us to actually npm publish google's repo, but just make a simple repo, with one index.ts that just says export * from '@google-cloud/bigquery' (and some other relevant wiring in package.json, etc).

It would also make package.json saner for the plugin server, as we could directly see which packages are there since they're extensions for plugins, and which are there since the plugin server itself needs them.

WDYT?

[yakkomajuri]

So just weighing in here given @macobo's tag.

For me, I've now accepted a bit of the "magic" of plugins, where some stuff is just available to me. It also helps because I do have some insight into how it works in the background, so it isn't entirely magic.

Hence, I like that I can use fetch and posthog "magically".

However, this is a bit confusing for users. I built a plugin with a user on a call and there was definitely confusion as to why some stuff he needed the meta for (metaphor?) whereas other things just existed.

Hence, my 2c (purely from a "plugin developer experience" light):

• global.fetch and global.posthog would feel weird to me
• I do however agree that the less "magic" the better
• Adding too many fields to the meta can get out of hand, but I'd argue for:
  • console.log completely global (like @macobo said)
  • posthog and fetch as their own meta props, so you can destructure them in the function definition and just use them "normally"
  • The rest of the stuff we're adding as part of global or some equivalent (lib?)

[mariusandra]

@yakkomajuri do you think ES imports could work and be easy to explain to users?

Do any of these options make sense?

// importing from our own stdlib
import { posthog } from '@posthog/plugin-library'

function runEveryMinute() {
  posthog.capture('haha')
}

// importing from a whitelisted "@posthog-plugins" package
import { bigquery } from '@posthog-plugins/google'

// importing natively, but again from a whitelisted source (e.g. @google-cloud/bigtable throws an error)
import { bigquery } from '@google-cloud/bigquery'

function processEvent(event) {
  bigquery.run('insert into events ?', event)
}

[yakkomajuri]

Yeah @mariusandra those make sense to me too - feels even less magic than meta

[mariusandra]

Ok, so I made a PR to add import support here: #284

After that, you can do:

import { BigQuery } from '@google-cloud/bigquery'
import fetch from 'node-fetch'

... and it'll just work.

Thus I propose:

• fetch and BigQuery and other whitelisted external libraries (mongodb? aws s3?)... we make available via imports. Their usage will look like it normally does, just we need to explain to users that only a select few places can be imported from. (TODO: add docs for the package names and versions)
• console, yup, that should be a "superglobal"
• posthog - what we expose here is not really a library, but just an object with one capture function that adds something onto kafka. Thus importing from posthog-js-lite or posthog-js is wrong, as it's not that (and there's no initing to be done to get this working, unlike the actual libraries). This could go into meta (possibly as meta.capture?), as it's not that dissimilar to cache and storage in that it's just a plugin server wrapper over another api.

[mariusandra]

Now that #284 is merged, this can be closed I think.