Skip to content

👷 Other strategy to handle test app lockfiles with renovate#4028

Merged
bcaudan merged 5 commits intomainfrom
bcaudan/revert-renovate-configs
Dec 16, 2025
Merged

👷 Other strategy to handle test app lockfiles with renovate#4028
bcaudan merged 5 commits intomainfrom
bcaudan/revert-renovate-configs

Conversation

@bcaudan
Copy link
Collaborator

@bcaudan bcaudan commented Dec 11, 2025

Motivation

Previous strategy #4019, #4021, #4023 did not work

Changes

  • Rollback previous strategy
  • Switch packages dependencies as optional peer dependencies
  • Install peer packages when building test apps

Test instructions

  • CI should pass
  • lock files maintenance should pass

Checklist

  • Tested locally
  • Tested on staging
  • Added unit tests for this change.
  • Added e2e/integration tests for this change.

@datadog-official
Copy link

datadog-official bot commented Dec 11, 2025

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage
Patch Coverage: 100.00%
Overall Coverage: 92.62% (+0.00%)

View detailed report

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: e1e9ff8 | Docs | Datadog PR Page | Was this helpful? Give us feedback!

@bcaudan bcaudan force-pushed the bcaudan/revert-renovate-configs branch from 4e8dab0 to 776b35f Compare December 11, 2025 15:24
@cit-pr-commenter
Copy link

cit-pr-commenter bot commented Dec 11, 2025

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum 164.30 KiB 164.30 KiB 0 B 0.00%
Rum Profiler 5.21 KiB 5.21 KiB 0 B 0.00%
Rum Recorder 19.74 KiB 19.74 KiB 0 B 0.00%
Logs 56.14 KiB 56.14 KiB 0 B 0.00%
Flagging 944 B 944 B 0 B 0.00%
Rum Slim 121.57 KiB 121.57 KiB 0 B 0.00%
Worker 23.63 KiB 23.63 KiB 0 B 0.00%
🚀 CPU Performance
Action Name Base CPU Time (ms) Local CPU Time (ms) 𝚫%
RUM - add global context 0.0051 0.0062 +21.57%
RUM - add action 0.0127 0.0197 +55.12%
RUM - add error 0.0115 0.0169 +46.96%
RUM - add timing 0.0028 0.0051 +82.14%
RUM - start view 0.0048 0.0053 +10.42%
RUM - start/stop session replay recording 0.0007 0.0011 +57.14%
Logs - log message 0.0138 0.0219 +58.70%
🧠 Memory Performance
Action Name Base Memory Consumption Local Memory Consumption 𝚫
RUM - add global context 25.70 KiB 25.51 KiB -198 B
RUM - add action 48.76 KiB 49.07 KiB +315 B
RUM - add timing 23.91 KiB 24.04 KiB +132 B
RUM - add error 53.67 KiB 54.63 KiB +979 B
RUM - start/stop session replay recording 23.49 KiB 23.13 KiB -360 B
RUM - start view 421.90 KiB 426.06 KiB +4.15 KiB
Logs - log message 44.81 KiB 45.32 KiB +520 B

🔗 RealWorld

@bcaudan bcaudan force-pushed the bcaudan/revert-renovate-configs branch from 776b35f to 5447b14 Compare December 11, 2025 15:49
@bcaudan bcaudan force-pushed the bcaudan/revert-renovate-configs branch from 5447b14 to 0bccecf Compare December 11, 2025 16:14
@bcaudan bcaudan marked this pull request as ready for review December 11, 2025 16:24
@bcaudan bcaudan requested a review from a team as a code owner December 11, 2025 16:24
Comment on lines +11 to +17
"peerDependenciesMeta": {
"@datadog/browser-logs": {
"optional": true
},
"@datadog/browser-rum": {
"optional": true
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

🥜 nitpick: If we're gonna install them anyway, then they are no optional (and it seems that npm would actually install them automatically, unfortunately this is not the case for yarn)

You might need to add the peer dependencies before doing yarn install in the build script though. I don't know if it's worth it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The issue is that renovate don't allow to build those dependencies before running the install to re-generate the lock file.

So the idea is to have a way to run the install without having those dependencies built, otherwise the install fails as the dependencies are missing.
But we still need to build them before effectively be able to use the test apps.

Do you have a simpler alternative in mind?

Copy link
Collaborator

Choose a reason for hiding this comment

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

all good then! thanks

printLog(`Building app at ${appPath}...`)
command`yarn install --no-immutable`.withCurrentWorkingDirectory(appPath).run()

// install peer dependencies if any
Copy link
Collaborator

Choose a reason for hiding this comment

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

💬 suggestion: ‏Maybe add a comment explaining why we are using peed dependencies instead of dependencies. I think I got it but it's not obvious. (ideally that comment would be in the package.json, but 🤷)

interruptible: true
script:
- yarn
- yarn build
Copy link
Collaborator

Choose a reason for hiding this comment

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

💭 thought: ‏I guess this is because otherwise the type definitions from the peer dependencies are not found? That's a bit annoying to have to build just to typecheck 😞 (but I don't see an alternative)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI, here I'm just reverting to the behavior that we add before #4019, since we don't nee to run yarn build at install anymore

@bcaudan bcaudan merged commit 35ca513 into main Dec 16, 2025
18 of 19 checks passed
@bcaudan bcaudan deleted the bcaudan/revert-renovate-configs branch December 16, 2025 09:52
@github-actions github-actions bot locked and limited conversation to collaborators Dec 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants