How should we handle provider APIs and their visibility?

[nekevss]

We need to deal with the provider API vs. non-provider API in a way that is ergonomic and
sane to deal with rather than keeping them in the same files (Mea culpa).

Currently, if we expose a non with_provider API, the with_provider method is still available. So if we make ZonedDateTime::to_ixdtf_string available then ZonedDateTime::to_ixdtf_string_with_provider will be available as well.

My initial thought is to separate the the two APIs into separate modules and then expose
them via feature flags.

General proposal overview

The general directory structure would be one of the two below.

NOTE: any bikeshedding on names would be fine by me. I like the core terminology for the
main implementation, but I'm not entirely convinced std is a good name. (Sort of mimicking
Rust's core vs. std separation).

.
+-- _src
|   +-- _core
|   +-- _std
|   +-- lib.rs

OR

.
+-- _src
|   +-- _builtins
|       +-- _core
|       +-- _std
|       +-- mod.rs
|   +-- lib.rs

The main idea would be to have all non-feature flagged APIs live in core. Meanwhile, for
all the general usage APIs (currently flagged "experimental") would be under std.

The general usage APIs would then just be either a reexport of the core impl or a
NewType of the core implementations if it has any provider APIs.

Other Options:

• Leave as is, with features in one file with both APIs exposed.
• Other suggestions

[jedel1043]

@nekevss Knowing that #169 is causing problems for the FFI, maybe we could use separate impl blocks instead of wrapper types? All our structs are in the same crate anyways.

[nekevss]

Yeah, I have a feeling that might be the best way to go about handling this.

That being said, the other solution is to remove "full" from being a default feature and have the wrapper types be opt-in. It prevents the problems with the FFI and stops any of the rust-analyzer workspace issues that happen when temporal_capi is set to default-feature = false.

[Manishearth]

I'd not use core vs std, it makes it feel like this is a no_std vs with_std distinction.

[nekevss]

I'd not use core vs std, it makes it feel like this is a no_std vs with_std distinction.

Agreed, that's why the initial change was core and native.

[nekevss]

The big reasons for the breaks here is that the previous implementation was greedy and tried to get away with using the core implementation of PlainYearMonth and PlainMonthDay. That's probably a mea culpa.

To fix it, the current implementation wrappers can be added for the PlainYearMonth and PlainMonthDay. that should shore up the major issues with the API from the native side, or we can go back to using impl blocks. The implementation blocks concern me because there will end up being two methods, i.e. round and round_with_provider on various types.

But adding the wrappers doesn't address the "opt-in" or "opt-out" behavior if the wrappers or kept.

Is there any preference for one approach over the other?

[Manishearth]

I'm still unclear on the purpose of this split; why it is implemented the way it is. If it's extra APIs, it should be implemented on the same type, right?

It's a violation of feature unification to hide APIs when a feature is enabled. Users of this crate may end up with breakages when found in a larger deptree.

Currently, if we expose a non with_provider API, the with_provider method is still available. So if we make ZonedDateTime::to_ixdtf_string available then ZonedDateTime::to_ixdtf_string_with_provider will be available as well.

This seems fine to me?

[jedel1043]

+1 for @Manishearth's comment. If having wrappers is just overcomplicating FFI and feature resolution, we should just return to separate impl blocks. I think we can have all the benefits of nicely delimited feature sets if we split the impl blocks into different modules, then flag the whole module as feature-gated.

[nekevss]

I'd be open to that idea. That would solve the issue with multiple feature flagged impl blocks all in one file.

Is there any preference as to how to handle API visibility. Do we forego attempting to separate with_provider methods from other methods? Or do we move ahead with just allowing the two to coexist?