Skip to content

feat(devex): vite#35247

Merged
adamleithp merged 29 commits intomasterfrom
devex/vite-2
Jul 23, 2025
Merged

feat(devex): vite#35247
adamleithp merged 29 commits intomasterfrom
devex/vite-2

Conversation

@adamleithp
Copy link
Copy Markdown
Contributor

@adamleithp adamleithp commented Jul 18, 2025

Problem

  • The time it currently takes for saving a file => refreshing the browser can range between engineers but usually > 2s until changes are visible. 2s is not the best, but bearable— some engineers report >10s, which is not acceptable.
  • The dev loop is unstable, any saving of a file intermittently causes the frontend script to crash
  • No HMR (hot module replacement)
  • insert other things..

This is working for developing on the app (see not tested).
This changes nothing about esbuild or it's build scripts, this should all work the same.

Known limitations

  • Kea - when working on any file that uses/imports a kea logic, a hard refresh will happen. IF you're working on a PURE react component, HMR is instant.

2025-07-10 06 03 40

Run

pnpm install && bin/start --vite

How it works

  • flag --vite tells bin/start to launch mprocs --config bin/mprocs-vite.yaml
  • which starts the mprocs, which is exactly the same as default, except:
    • we set env variable POSTHOG_USE_VITE: '1'
    • launch /bin/start-frontend-vite
    • then finally bin/turbo --filter=@posthog/frontend start-vite which...
  • Runs vite.config.ts:
    • copy over html from src/ => dist/ (without ESBUILD scripts/tags in the HTML)
    • copy public => frontend/src/assets/ (so vite doesn't complain)
    • start dev server (http://localhost:8010)

Not tested

  • Toolbar (in a follow up PR)
  • EE (in a follow up PR)

If you need to work on these things, it's best to use normal ESBUILD

Changes

  • Can no longer use require() inside the frontend (storybook stories are fine as this still uses webpack)

How did i test this?

Vite (local)

  • rm -rf **/node_modules
  • pnpm install
  • bin/start --vite
  • visit http://localhost:8010/ as normal ⚡
  • Wait for EVERY js file to be loaded only first load (~20s)
  • 🚀

Normal ESBUILD (start/local)

  • bin/start
  • working
  • visit http://localhost:8010/ as normal

Normal ESBUILD (build/local)

  • cd frontend && pnpm build
  • working

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR introduces Vite as an alternative build tool to webpack to improve development speed. The changes include:

  1. New Vite configuration and plugins:

    • Added vite.config.ts with React support and CORS configuration
    • Custom plugins for HTML generation and Kea typegen support
    • Proper asset handling for Lottie animations and JSON files
  2. Infrastructure changes:

    • New scripts: bin/start-frontend-vite and mprocs-vite.yaml
    • Modified bin/start to support --vite flag
    • Updated package.json with Vite dependencies
  3. Frontend adaptations:

    • TypeScript module declarations for Vite's URL imports
    • Template modifications in head.html for Vite development server
    • Barrel file pattern adoption in some components

The changes maintain backward compatibility by keeping the existing webpack setup while providing an opt-in path for using Vite.

Confidence score: 9/5 (3/5)

  1. This PR is moderately safe to merge as it adds functionality without removing existing build system
  2. Score reflects concerns about error handling in HTML plugin and potential environment-specific issues
  3. Files needing attention:
    • frontend/vite-html-plugin.ts: Error handling needs improvement
    • frontend/src/lib/animations/animations.ts: Type 'any' usage should be reviewed
    • posthog/templates/head.html: Hardcoded development server port

11 files reviewed, 11 comments
Edit PR Review Bot Settings | Greptile

Comment thread frontend/src/lib/animations/animations.ts
Comment thread posthog/templates/head.html Outdated

{% if debug %}
<script type="module">
import RefreshRuntime from 'http://localhost:8234/@react-refresh'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider using a configuration variable for the Vite dev server port (8234) instead of hardcoding it in multiple places

Comment thread bin/start-frontend-vite Outdated
Comment thread frontend/src/custom.d.ts
Comment thread frontend/src/custom.d.ts
Comment thread frontend/vite-kea-typegen-plugin.ts Outdated
Comment thread bin/mprocs-vite.yaml
Comment thread bin/mprocs-vite.yaml
Comment thread frontend/vite-html-plugin.ts Outdated
Comment thread frontend/vite-html-plugin.ts
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 18, 2025

Size Change: -60 B (0%)

Total Size: 2.62 MB

ℹ️ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 2.62 MB -60 B (0%)

compressed-size-action

Comment thread bin/start-frontend-vite Outdated
Copy link
Copy Markdown
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gonna try it, but for now one comment inline

Comment thread bin/start Outdated
Comment on lines 61 to 62
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of all of this, one other option is to just add frontend-vite to the default mprocs, with autostart: false. Then whoever wants can opt in by turning the default off

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that's a good idea so that we don't have to keep two very similar mprocs in sync, but ideally this is merged, tested as experimental (not really ready for all devs), and once we can ship features on top of it, we remove the flag + other mprocs.

Comment thread frontend/src/custom.d.ts Outdated
@mariusandra
Copy link
Copy Markdown
Collaborator

When running, I seem to get a blank screen and this in the console:

image

I also see a bunch of errors from window.ESBUILD_LOAD_CHUNKS... which is something esbuild added. 🤔

@adamleithp adamleithp force-pushed the devex/vite-2 branch 2 times, most recently from 15a9ffe to 34c23c3 Compare July 18, 2025 15:25
@posthog-bot
Copy link
Copy Markdown
Contributor

📸 UI snapshots have been updated

18 snapshot changes in total. 0 added, 18 modified, 0 deleted:

  • chromium: 0 added, 18 modified, 0 deleted (diff for shard 12)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vite complained about it being a .js file

key: SDKKey.NEXT_JS,
tags: [SDKTag.WEB, SDKTag.RECOMMENDED],
recommended: true,
image: require('./logos/nextjs.svg'),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't use require in vite without some bloated commonjs plugin

export function htmlGenerationPlugin(): Plugin {
return {
name: 'html-generation',
buildStart() {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove this buildStart as we don't ever build in this scenario

Comment thread frontend/vite.config.ts
configureServer(server) {
server.httpServer?.once('listening', () => {
setTimeout(() => {}, 100)
})
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: output a log to the terminal about visiting 'localhost:8010', currently only shows "visit localhost:8xxx" which is the vite dev server, not what you should visit.

@benjackwhite benjackwhite removed their request for review July 21, 2025 06:58
Copy link
Copy Markdown
Contributor

@joshsny joshsny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woohooo 🙌 This is much faster for me and was a great experience when i gave it a trial run, thank you for working on it!

Will leave the stamp for @mariusandra since he's most familiar with the current build process - once merged I'll use it locally when developing and let you know if I encounter any snags.

@adamleithp adamleithp removed the request for review from zlwaterfield July 21, 2025 08:01
Copy link
Copy Markdown
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a look and found one more require() that was throwing a lot of errors. Pinging Tom to see if the workaround is worth it. Other than that, I'm also not sure how or why the esbuild utils.mjs is used. I remember that my index.html got a lot of crap in it... but where did it come from, if we should totally be using vite now?

Still some typescript errors to fix. I'm not sure what's causing them - had a 10min attempt, and managed to fix something by changing a type from JSX.Element to React.ReactNode, but it felt a bit like magic... so not sure 🤔

Comment thread frontend/vite.config.ts Outdated
Comment thread frontend/src/custom.d.ts Outdated
Comment thread common/esbuilder/utils.mjs Outdated
Comment thread frontend/src/queries/utils.ts Outdated
@posthog-bot
Copy link
Copy Markdown
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 14)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Copy Markdown
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 14)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@adamleithp adamleithp force-pushed the devex/vite-2 branch 3 times, most recently from 980788b to 725fc1b Compare July 22, 2025 15:13
@posthog-bot
Copy link
Copy Markdown
Contributor

📸 UI snapshots have been updated

20 snapshot changes in total. 0 added, 20 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@adamleithp
Copy link
Copy Markdown
Contributor Author

adamleithp commented Jul 22, 2025

  • Type errors gone (turbo script was needed to do build, which handled a bunch of ts configurations)
  • No more VITE warning about assets (in vite we copy over public => frontend/src/assets, which we also gitignore)
  • No more ESBUILD artifacts in HTML (we delete and re-copy over index.html files from src/ => dist)
  • Updated PR to include known limitation (kea hmr)
    cc @mariusandra

@adamleithp adamleithp requested a review from mariusandra July 23, 2025 05:40
Copy link
Copy Markdown
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@adamleithp adamleithp merged commit 6420d68 into master Jul 23, 2025
160 checks passed
@adamleithp adamleithp deleted the devex/vite-2 branch July 23, 2025 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants