Skip to content

fix(ci): update collectstatic cp path after rrweb migration#59839

Closed
mendral-app[bot] wants to merge 1 commit into
masterfrom
mendral/fix-collectstatic-cp-path
Closed

fix(ci): update collectstatic cp path after rrweb migration#59839
mendral-app[bot] wants to merge 1 commit into
masterfrom
mendral/fix-collectstatic-cp-path

Conversation

@mendral-app
Copy link
Copy Markdown
Contributor

@mendral-app mendral-app Bot commented May 24, 2026

Summary

  • Updates the E2E Playwright workflow's "Collect static files" step to reference posthog-js/dist instead of the removed @posthog/rrweb/dist package
  • Adds graceful error handling (|| true) to match the try-catch logic in common/esbuilder/utils.mjs

Context

PR #59518 removed @posthog/rrweb as a direct dependency but forgot to update the E2E CI workflow's cp command, causing the "Collect static files" step to fail with exit code 1.

Related insight: https://app.mendral.com/insights/01KSD2PEJAB83MEWGXRQ4N15KR


Note

Created by Mendral. Tag @mendral-app with feedback or questions.

@mendral-app mendral-app Bot requested a review from arnohillen May 24, 2026 14:02
@arnohillen
Copy link
Copy Markdown
Contributor

We've got a competing fix on ahillen/drop-playwright-rrweb-cp (commit 0ef0c10) that takes a different approach: drop the cp entirely rather than re-pointing it at posthog-js/dist.

Rationale: frontend/build.mjs already calls copyRRWebWorkerFiles() from common/esbuilder/utils.mjs, which mirrors node_modules/posthog-js/dist/image-bitmap-data-url-worker-*.js.map into frontend/dist/ as part of pnpm --filter=@posthog/frontend build — the step immediately preceding Collect static files. So the workflow's cp was always duplicating bundler logic; PR #59518 just made that duplication visible by breaking the stale path.

This PR's 2>/dev/null || true works but (a) duplicates the path between YAML and utils.mjs, and (b) silently swallows failures, so if the rrweb files ever move again we won't notice until collectstatic fails much later.

Happy to close the alt branch if you'd rather merge this one, but flagging in case you want to close #59839 in favor of the leaner fix.

@arnohillen
Copy link
Copy Markdown
Contributor

Closing in favor of #59840, which takes the "drop the cp entirely" approach — frontend/build.mjs's copyRRWebWorkerFiles() (in common/esbuilder/utils.mjs) already mirrors the worker maps from posthog-js/dist into frontend/dist/ during pnpm --filter=@posthog/frontend build, so the workflow-level cp is redundant once we trust the bundler.

Thanks @Mendral — the diagnosis and remediation pointer here were spot on, just took the leaner branch on the implementation. 🙏

@arnohillen arnohillen closed this May 24, 2026
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.

1 participant