Stop re-exporting packages to avoid breaking changes

[provokateurin]

When we re-export another package then the breaking changes of that package also become our breaking changes.

[provokateurin]

I wonder if this actually makes sense. Do we consider updating a dependency with a breaking change to be a breaking change for our users as well? From the user perspective it doesn't matter if we re-export a package and the package has a breaking change or we update a dependency and pull in a breaking change. We could mitigate it by using version ranges, but that is a lot of maintenance and error-prone. I'm not sure if renovate correctly adds the ! when the update is a major version bump (but we can probably configure that). @Leptopoda what do you think?

[provokateurin]

I couldn't find an option to configure the ! in the commit messages with renovate and there was also no relevant issue to the problem.

[provokateurin]

Either way we need to mark major dependency updates as breaking changes. Afterwards we can still decided if we want to keep re-exporting the packages. I'd be in favor of that because it makes it easier for consumers.

[Leptopoda]

I would not consider both cases the same. See this discussion in the dependency_validator issue tracker (https://github.com/Workiva/dependency_validator/issues/6). The person (from the dart-core team) argues that in cases you depend on the interface of a transitive dependency you should just add it to your pubspec to avoid such issues.

A breakdown on all packages (correct me if I missed something):

• nextcloud (export of dynamite_runtime) it remains to be seen how often we need to bump the runtime but we should consider it an integral part of the generated package. In that case we should probably stay with the mentality of "breaking dynamite change should result in a breaking NC bump". So we could (any maybe should) keep the export.

• dynamite_runtime (export of cookie_jar) The cookie jar is only used by the nextcloud_test (link) package and client (should be switched to a direct dependency and import). And by the Account in the neon_framework (link). Here I don't know what we should do. Maybe we should handle it similar to the httpclient where the DynamiteClient instantiates a sensible default with the ability to let apps overwrite it with a custom implementation. Is there a usecase where a user should not persist cookies at all? If so they could just provide a dummy Jar that discards the operations.

• neon_framework (export of sort_box) This is a tricky question. We develop this package ourselves and probably do not expect many breaking changes as the api is quite small.

Either way we need to mark major dependency updates as breaking changes.

See the first link. I do not think this makes sense and it is not what the rest of the dart ecosystem does.

[provokateurin]

Thanks for your input.

In dynamite_runtime we could change how we handle cookies and only provide a boolean to enable or disable cookies. I don't see why anyone would provide a custom implementation anyway. This would solve the problem IMO and not cause any problems in the future.

I think for packages that we re-export but also develop in this repo we actually do everything correct already, because a commit like fix(dynamite_runtime)!: bla bla would also contain the necessary changes for the nextcloud package. Then that commit would also show up in the history of the nextcloud package and correctly detected as a breaking change.

[Leptopoda]

I don't see why anyone would provide a custom implementation anyway.

Cookies are currently only persisted in memory and some implementations might require persisting them across runs in a database.

[provokateurin]

Oh that is actually wrong, the cookies should have been saved on disk by default. But yeah maybe memory-only cookies are useful in some cases.

[provokateurin]

So what do we do with cookie_jar? Just stop re-exporting and adding it as a dependency everywhere?

[Leptopoda]

I'd make it similar to the httClient where the DynamiteClient instantiates it as a PersistCookieJar to fix the above limitation with an option to overwrite it. That way our unit tests could still use the in memory jar.

Do you have the bandwidth to make this or should I quickly make a PR?

[provokateurin]

Adding a default is good, but still: should we keep re-exporting it or not? Also for neon_framework we still need to use the memory jar for now: https://github.com/nextcloud/neon/issues/1536#issuecomment-1913109469

Please create the PR if possible.

[provokateurin]

After https://github.com/nextcloud/neon/pull/1537 is merged we can close the issue, right? For our own packages we know that the breaking change will always be communicated correctly.