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

Introduce @tus/utils to simplify building/publishing #567

Merged
merged 4 commits into from
Feb 5, 2024
Merged

Conversation

Murderlon
Copy link
Member

@Murderlon Murderlon commented Jan 29, 2024

Problems:

  • In all packages we have @tus/server in peerDependencies but also in devDependencies (for tests).
  • peerDependencies changes aren't noticed by package install unless it's a major. This is why changeset (our versioning and release tool) always bumps all stores to a major for any kind of release to server. See Version Packages #566
  • Turbo can't cache correctly when running tests. For instance, file-store depends on server but server tests depend on file-store. We can't create a circular dependency so we omit file-store as dev but then changes aren't picked up.

Solution:

  • Introduce @tus/utils, have it in dependencies everywhere. All things that are shared between all packages are in here. Remove @tus/server from peerDependencies everywhere.
  • No breaking changes in exports.

Now we have correct dependency graphs for turbo and we can publish more correctly and easily with changesets.

This is arguably a breaking change, but I'd argue it isn't. It only is if you update one package but not the other. However that can happen all time the time in monorepo setups like this. Whether it's a bug fix that is coordinated between packages, or a feature that needs detecting in a store, you can't expect all changes to work if you don't upgrade packages together. In Uppy, our policy is it's not a breaking change if we don't change exports and no public methods and properties a are changed.

@Murderlon Murderlon self-assigned this Jan 29, 2024
@Acconut
Copy link
Member

Acconut commented Jan 30, 2024

I don't understand how @tus/utils helps with the peerDependencies issue. Can you elaborate on this? Or is not addressed by this PR?

@Murderlon
Copy link
Member Author

Murderlon commented Jan 30, 2024

As seen in the diff, there are no peer dependencies anymore and thus no problems anymore associated with them. All packages now have @tus/utils in dependencies. Similar to how this was solved in Uppy with @uppy/utils.

@Murderlon Murderlon merged commit a896d25 into main Feb 5, 2024
4 checks passed
@Murderlon Murderlon deleted the @tus/utils branch February 5, 2024 10:36
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.

None yet

2 participants