Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bumping dependencies on publish #100

Closed
jeroenptrs opened this issue Jun 27, 2020 · 25 comments
Closed

Bumping dependencies on publish #100

jeroenptrs opened this issue Jun 27, 2020 · 25 comments

Comments

@jeroenptrs
Copy link
Contributor

jeroenptrs commented Jun 27, 2020

I'm writing an application in a monorepo where a specific package is using another. Upon publishing I just realized that the version of that package in the dependencies field doesn't get bumped.

Am I missing something? Otherwise I'd like to request that feature (and potentially running yarn build in between each publish, you can easily configure this with a prepare script btw)

@jeroenptrs
Copy link
Contributor Author

I made a tiny package that does just this to quickly solve my needs as I'm not sure this is something you want to support.
@guigrpa, if it's something you think fits in oao I'll port it :)
https://github.com/jeroenptrs/bump-siblings

@onigoetz
Copy link

onigoetz commented Jul 7, 2020

Hi,

I'd definitely be interested in this as well, I'm looking for a lightweight alternative to Lerna (which had this feature) and it would be a showstopper to not have it :/

@jeroenptrs
Copy link
Contributor Author

Right now I run my package under the postpublish script in package.json (which gets auto triggered when you publish a package).

The only issue I still have is that the actual bumping (with npm publish) also commits before any of the changes, so the actual bumping of packages is left unstaged. You could run oao publish with the --no-git-commit flag and then commit everything in one go (or like I did, force push and re-set the tag)

@onigoetz
Copy link

onigoetz commented Jul 8, 2020

So it's only accurate on the next release ?

If I have a package a:1.0.0 that depends to b:1.0.0 If I publish version 1.1.0 then a:1.1.0 will depend on b:1.0.0, right ?
Shouldn't that change be done pre-publish ?

@jeroenptrs
Copy link
Contributor Author

This issue and #101 really go hand in hand -- but a PR for that issue is already up at #102. The way you want this to happen is to follow the reverse dependency tree. So that b (the package a depends on) gets published first. Then the postpublish hook kicks in, bumping the version of b in a, and then a gets published. This is obviously not a use case when a and b depend on each other, but I'm not a fan of organizing my packages that way.

What I'm hinting at is that oao runs npm version first which bumps the version of each package and then commits that change. It's just a bit of manual work but if you want to commit all changes in one go you could run oao publish --no-git-commit (with the bump-siblings package as a postpublish hook in all packages that are a dependency in others) and commit it all together yourself. It's obviously a bit hacked together for the sake of proof for this issue but it does work :)

note: I mistakenly said npm publish in my previous comment, but it's npm version then npm publish per package.

@onigoetz
Copy link

onigoetz commented Jul 8, 2020

Okay I see, I'm not really sure this has to be done in a particular order, as long as the version is updated at publish time.

It could also change the list of package to publish; If the version needs to be updated; the package needs to be published.

I would see it working this way:

  • Find changed repositories (personally I would be fan of a --all option to publish all packages everytime, but that's for another issue/PR :) ) add them to the list of repositories to publish
  • Find repositories that depend on others and add them as well to the list
  • Ask for new version (or take from CLI args)
  • Apply version everywhere, including dependencies
  • commit/push if needed (!--no-git-commit)
  • publish if needed (!--no-publish)

@jeroenptrs
Copy link
Contributor Author

I agree that (sub)dependency checking should happen at a more sensible step. Ideally this should all be part of prepublish checks!

@jeroenptrs
Copy link
Contributor Author

jeroenptrs commented Jul 8, 2020

personally I would be fan of a --all option to publish all packages everytime, but that's for another issue/PR

I guess you could say that's how it is now, by default

@onigoetz
Copy link

onigoetz commented Jul 8, 2020

I guess you could say that's how it is now, by default

I'm confused, it publishes all packages everytime ?
If yes then why do the check of changes since last release ?

If it's the default that's great anyway :)

@guigrpa
Copy link
Owner

guigrpa commented Jul 18, 2020

Thanks for all the interesting contributions.

I've also been bitten by oao's not bumping cross-dependency versions, which generates some hard-to-find bugs later on.

In fact, in order to simplify oao publish (there are already other more complex tools out there when you need a lot of bells and whistles), we could:

  1. Publish in reverse-dependency order by default (i.e. no need to specify --tree — see chore: use reverse dependency tree in oao publish #102)
  2. Always publish all dependencies, with the version number specified by the user. One of oao's premises is that subpackage versions are synchronised, so this would make the tool more predictable
  3. Before publishing as version x.y.z, always bump all internal dependencies (among subpackages in the monorepo) to ^x.y.z

At least 2 and 3 would be breaking changes. But I think the API surface is reduced, the tool becomes simpler and more predictable (and more maintainable as well 😀).

@onigoetz
Copy link

  1. I don't know the exact implication of doing or not doing it
  2. I totally agree with it, that's what I do in my projects anyway.
  3. I'd give the option to use range or exact, could be open to the user. the default could be the range since that's what is more common in the ecosystem it seems.

And yes a simpler and more predictable tool is exactly what I'm looking for coming from Lerna :) (no hate on Lerna, it's a great tool as well :) )

@jeroenptrs
Copy link
Contributor Author

I think 1 is also how lerna does it by default, since you'd want to start your workflow at the leaves and build your way up. @guigrpa if you wan't I'll do this in my PR :)

@jeroenptrs
Copy link
Contributor Author

As far as the rest I'm ok with that. Especially 2) is how I do it now at the moment too !

@guigrpa
Copy link
Owner

guigrpa commented Jul 19, 2020

@jeroenptrs Yes, please set the reverse-dependency order as default (removing the flag). Thanks!

@onigoetz
Copy link

I can modify my PR ( #104 ) to always take all packages and change all their dependencies, this would solve 2) and 3)

@guigrpa
Copy link
Owner

guigrpa commented Jul 19, 2020

@onigoetz That's very helpful, thanks! I guess the new approach would remove most of the complexity in the PR, right? For instance determining which subpackages are dirty or not — all are considered dirty.

Regarding your suggestion on how to modify dependent versions, we could have: 1) --update-dependent-versions (with a range, ^x.y.zdefault), 2) --update-dependent-versions-exact (with the new monorepo version, x.y.z), and 3) --no-update-dependent-versions.

@guigrpa
Copy link
Owner

guigrpa commented Jul 19, 2020

The flag names are still a bit confusing to me. What would we call this operation? Bumping dependent requirements maybe? Then maybe the most appropriate names could be: --bump-dependent-reqs, --bump-dependent-reqs-exact, --no-bump-dependent-reqs.

@onigoetz
Copy link

Yes it would indeed remove much of its complexity :)

Could it also be a value instead of different options ?

  1. --update-dependent-versions= "range" | "exact" | "no" (with range being the default)
  2. --bump-dependent-reqs= "range" | "exact" | "no"
  3. --bump-dependent-versions= "range" | "exact" | "no"

@jeroenptrs
Copy link
Contributor Author

jeroenptrs commented Jul 19, 2020

I feel like it'd be smarter to do this implicitly, setting the version on publish could tell whether it's range or exact?
--new-version ^x.y.z would mean we're bumping with a range, --new-version x.y.z would mean we're bumping to an exact version. And if you want users to explicitly state dependencies shouldn't be bumped in that case you can add a flag --no-dependents

@onigoetz
Copy link

@jeroenptrs that wouldn't work if the version is requested by the CLI instead of provided as an argument

@jeroenptrs
Copy link
Contributor Author

jeroenptrs commented Jul 19, 2020

@onigoetz It would indeed not work on --increment-version-by as we have it right now, but I think we can make it work.

a) As seen here: https://github.com/guigrpa/oao/blob/master/src/publish.js#L94..L97 the next version is taken either from prompt or cli commands. But both the prompt and --new-version could already take interpret an implication with minimal changes (just looking for tilde or carot -> range, else exact).
b) I feel like we're mostly adding flags to override default behaviour, so I'd say the value that comes out of --increment-version-by could just follow default behaviour (use range and do update dependents) and if the user wants to override you can use flags like --no-dependents and --exact.

a - edit) as suggested in b) we can also always imply a range and override with exact.

This seems the most straightforward approach in terms of UX, especially since oao is already opinionated I don't see any harm in doing it this way. What do you think? @onigoetz @guigrpa

@guigrpa
Copy link
Owner

guigrpa commented Jul 20, 2020

Could it also be a value instead of different options ?

@onigoetz Certainly! I prefer in this case --bump-dependent-reqs= 'range' | 'exact' | 'no' (default: range). The other options refer to dependent-versions, but I find that misleading. It's not the versions of dependent packages that we're modifying, but their version requirements.

I feel like it'd be smarter to do this implicitly, setting the version on publish could tell whether it's range or exact?
--new-version ^x.y.z would mean we're bumping with a range, --new-version x.y.z

Asking the user to introduce the carot in --new-version for what we consider default behaviour is a bit strange. If the user doesn't read this, she'd have --new-version x.y.z meaning "use exact versioning for dependent reqs" and --increment-version-by major meaning "use range versioning for dependent reqs". It looks confusing to me.

I would not couple how the user sets the new version to how oao bumps package requirements. It may save a few keystrokes, but makes the UX a bit more cryptic, in my opinion. Here are some examples how I would see it:

  • oao publish (default [opinionated] behaviour, bumps all subpackages to the version set by the user interactively + bumps dependent reqs + publishes all)
  • oao publish --bump-dependent-reqs exact or no (same, but with a different configuration for dependent reqs)
  • oao publish --new-version x.y.z (no need to prompt + dependent reqs using ranges)
  • oao publish --new-version x.y.z --bump-dependent-reqs exact or no (same, but using exact dependent reqs)
  • oao publish --increment-version-by major (no need to prompt + dependent reqs using ranges)
  • oao publish --increment-version-by major --bump-dependent-reqs exact or no (no need to prompt + exact dependent reqs)
  • oao publish --no-bump (doesn't touch versions nor dependent reqs)

@jeroenptrs
Copy link
Contributor Author

jeroenptrs commented Jul 20, 2020

I think instead of --bump-dependent-reqs <exact|no> it might be more straightforward to use something like --no-bump-dependents and --bump-exact? There's something about the terminology in the proposed flag that doesn't sit well, separate flags make more sense imo here. But in the end I'm partial to either solution, I'll probably stick with bumping in range anyhow 😝

@onigoetz
Copy link

Okay, I updated my pull request on #104

@guigrpa
Copy link
Owner

guigrpa commented Jul 25, 2020

This has shipped in v2.0.0. Please let me know if you find any issues!

Thanks for all the ideas and work put behind this issue and related PRs.

@guigrpa guigrpa closed this as completed Jul 25, 2020
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

No branches or pull requests

3 participants