[RUMF-1357] Add a peer dependency between rum and logs packages

[agrobbin]

Motivation

I suggested this, so made the PR too! This closes #1666.

Changes

The RUM docs (here and here) state that it is recommended to match versions, so this seems like a beneficial addition!

I also believe (though I'm having a hard time fully parsing the Dependabot source to be sure) it will force Dependabot to update both @datadog/rum and @datadog/logs in tandem if both are included dependencies of a project.

Testing

My primary concern from a "testing" perspective is whether Lerna will know to update these when a new version is released. I'm not familiar w/ Lerna, or the configuration for it, but hopefully this is a starting point!

[bits-bot]

All committers have signed the CLA.

[BenoitZugmeyer]

Thank you for your PR! Could you make those peer dependencies "optional" with peerDependenciesMeta?

As you guessed, lerna will be an issue for that. They intentionally removed support for it, and closed a feature request asking for an option to bump them on publish. I think we'll need to tweak our version script to bump it separately.

You can try it yourself with something like npx lerna version --no-push --no-git-tag-version -y major

[agrobbin]

@BenoitZugmeyer good call, I just added peerDependenciesMeta. I'm not sure what the idea might be re: the cli script. It'd be ideal to programmatically manipulate the contents of package.json, but I'm not terribly familiar with that landscape. A quick Google suggests npe, json, and a few others, but I'm definitely open to some suggestions here!

Additionally, after reading some of the comments in lerna/lerna#1575, I do want to confirm that this is the correct usage of peerDependencies. I still think it makes sense, but this isn't my project. 😆

[BenoitZugmeyer]

I just added peerDependenciesMeta.

Thank you!

I'm not sure what the idea might be re: the cli script. It'd be ideal to programmatically manipulate the contents of package.json, but I'm not terribly familiar with that landscape. A quick Google suggests npe, json, and a few others, but I'm definitely open to some suggestions here!

Mmh I was thinking about a simple "find and replace" script (ex: with sed), or if we want to do something fancier, a 3 lines nodejs script like:

const pkg = JSON.parse(fs.readFileSync('package.json'))
pkg.peerDependencies["@browser-logs"] = pkg.version
fs.writeFileSync('package.json', JSON.stringify(pkg, null, 2))

But don't worry about that, we can take over your PR to make the final adjustments 🙂

Additionally, after reading some of the comments in lerna/lerna#1575, I do want to confirm that this is the correct usage of peerDependencies. I still think it makes sense, but this isn't my project. 😆

I agree that it still makes sense for us. I think arguments are a bit outdated, for example “peerDependencies themselves only warn, not auto-install” is not true anymore: it either don't warn (optional) or auto-install (required). It made sense when peer dependencies were kind of required, but we want our packages to be optional.

[agrobbin]

@BenoitZugmeyer ah got it, that makes sense! I've enabled maintainer editing access on my branch, which hopefully is what you need to finalize the bin/cli script changes. Let me know if there is anything else I need to do.

[agrobbin]

@BenoitZugmeyer love it!

[liywjl]

Nice work!

[liywjl]

should we use JSDoc?

[BenoitZugmeyer]

Thank you again for your contribution @agrobbin , this will be released shortly.

[agrobbin]

Awesome, thanks @BenoitZugmeyer!

[agrobbin]

Well, looks like I was wrong about Dependabot bumping these together. 😢