Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a5f9cb9ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const bundleMtime = fs.statSync(bundlePath).mtimeMs | ||
|
|
||
| return fs.globSync('src/**/*', { cwd: WORKER_PATH }).some((file) => { |
There was a problem hiding this comment.
Rebuild worker when non-src inputs change
needsWorkerRebuild() only compares bundle/worker.js against files under packages/worker/src, but the worker bundle also depends on inputs outside that tree (for example @datadog/browser-core imported in packages/worker/src/boot/startWorker.ts via tsconfig path aliases, and webpack settings used by scripts/build/build-package.ts). After changing those external inputs, this check can incorrectly skip rebuilding and WORKER_STRING will embed stale worker code in RUM builds/tests until someone manually rebuilds or deletes the bundle.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
True but the function imparted in startWorker.ts is minimal, it should be fine.
There was a problem hiding this comment.
I guess this will also only happen in local environment. In the CI it will always be build, so we don't risk shipping stale worker in a release.
|
|
||
| const bundleMtime = fs.statSync(bundlePath).mtimeMs | ||
|
|
||
| return fs.globSync('src/**/*', { cwd: WORKER_PATH }).some((file) => { |
There was a problem hiding this comment.
I guess this will also only happen in local environment. In the CI it will always be build, so we don't risk shipping stale worker in a release.
Motivation
Build is unnecessary slow because we rebuild the worker unnecessarily most of the time.
Changes
Before
After
Test instructions
Tests should cover this change, however you can try the commands above
Checklist