Reinstate Posthog analytics PR fixing type definitions via installing dev dependencies #6532
Conversation
This reverts commit c5ea253.
Remove definitions for sentry and rrweb-snapshot
novocaine
force-pushed
the
posthog-analytics
branch
from
af3f319 to
c66d001
Compare
|
@novocaine it seems that for some reason the CI does not patch the packages properly, or it's using a cached version. |
|
It seems that because we run |
|
@novocaine I guess we can just run it manually then? |
|
We could, but then we're trusting I considered just running |
This is neccessary to resolve re-exported types referred to by posthog-js' type definitions. This isn't ideal, but * We intend to start using sentry anyway * Discussion with posthog maintainers about rrweb-snapshot at PostHog/posthog-js#252, perhaps we can find another solution soon
|
Giving up and adding them as dev dependencies in the interests of pragmatism. As noted in the commit we will shortly be using sentry anyway, and I'm chasing up the posthog maintainers at PostHog/posthog-js#252 |
novocaine
changed the title
Part of the previous PR for posthog #6495 included overriding posthog's type definitions to remove references to transitive dependencies which we don't use and don't want to install. This was done via a
pathsentry intsconfig.json.This caused element-web linting to fail as EW's type linting descends into matrix-react-sdk but the override didn't apply to EW as no changes were made to its respective
tsconfig.json. So I reverted the PR in #6531.It wouldn't be great for every Typescript user of
matrix-react-sdkto have to do that.In this PR, I initially took a different approach suggested by @Palid - use
patch-packageto autopatch the library on postinstall. Unfortunately, as noted in the comments, we disable the postinstall hook in CI, and it would be awkward to work around that.I considered forking the library, but we don't currently maintain any forked dependencies and it seems silly to start doing that now for this one.
So for the time being, we are just going to install the libraries referred to as dev dependencies. We will be using Sentry shortly anyway. Flagged with posthog maintainers at PostHog/posthog-js#252