-
Notifications
You must be signed in to change notification settings - Fork 3.4k
chore(toolbar): drop lowlight all-grammar re-export and posthog-typed typings from bundle graph #72760
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
Closed
haacked
wants to merge
1
commit into
master
from
posthog-code/toolbar-graph-trim-lowlight-posthog-typed
+83
−0
Closed
chore(toolbar): drop lowlight all-grammar re-export and posthog-typed typings from bundle graph #72760
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import type { CaptureOptions, CaptureResult, Properties } from 'posthog-js' | ||
| import originalPostHog from 'posthog-js' | ||
|
|
||
| // Toolbar shim — lib/posthog-typed is ~200 KB of generated event typings wrapped around a | ||
| // small runtime. This mirrors that runtime exactly (capture/captureRaw delegating to the | ||
| // posthog-js singleton, everything else proxied through) without the typings. | ||
| const enhanced: Record<string, unknown> = { | ||
| capture: ( | ||
| event_name: string, | ||
| properties?: Properties | null, | ||
| options?: CaptureOptions | ||
| ): CaptureResult | undefined => originalPostHog.capture(event_name, properties, options), | ||
| captureRaw: ( | ||
| event_name: string, | ||
| properties?: Properties | null, | ||
| options?: CaptureOptions | ||
| ): CaptureResult | undefined => originalPostHog.capture(event_name, properties, options), | ||
| } | ||
|
|
||
| const posthog = new Proxy(enhanced, { | ||
| get(target, prop) { | ||
| if (prop in target) { | ||
| return target[prop as string] | ||
| } | ||
| return (originalPostHog as unknown as Record<string | symbol, unknown>)[prop] | ||
| }, | ||
| set(_target, prop, value) { | ||
| ;(originalPostHog as unknown as Record<string | symbol, unknown>)[prop] = value | ||
| return true | ||
| }, | ||
| }) as unknown as typeof originalPostHog & { captureRaw: typeof originalPostHog.capture } | ||
|
|
||
| export default posthog | ||
|
|
||
| // Re-export everything else from posthog-js, matching lib/posthog-typed's surface | ||
| export * from 'posthog-js' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The best question here is: why is lowlight being pulled here at all? Can we simply load lowlight lazily wherever it's being loaded? This way it will not be added to the same bundle as the toolbar
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.
Good news: lowlight is already lazy. The product tours code loads it through a dynamic import, so
esbuildsplits the wholetiptap/lowlightchain into its own chunk (~662 KB) that only downloads when someone actually saves or previews a tour step in the toolbar. It never ships on page load.I also measured what this PR actually saves in shipped bytes: 90 bytes eager, 0 deferred. The entire 1.33 MB "win" was in the pre-tree-shake input metric, which is exactly what #72583 deletes. Tree shaking was already throwing all of this away.
So this PR trades a shim, a parity test, and a build hack for 90 bytes. Not worth it. Closing in favor of #72583.