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

How deeply should we integrate with the ecosystem? #27

Closed
Pauan opened this issue Mar 16, 2019 · 15 comments
Closed

How deeply should we integrate with the ecosystem? #27

Pauan opened this issue Mar 16, 2019 · 15 comments

Comments

@Pauan
Copy link
Contributor

Pauan commented Mar 16, 2019

Right now web-sys provides very low-level access to the web APIs. My understanding is that Gloo is intended to create idiomatic high-level Rust APIs on top of those low-level APIs. However, there are different degrees of "high-level".

As an example, it's easy to wrap setTimeout and setInterval to make a nice convenient API that uses FnOnce and FnMut. But that's only slightly more high-level than using the APIs directly.

On the other hand, we could wrap setTimeout and setInterval into a Future and Stream, which is definitely much higher level (and more idiomatic for Rust).

So when we are faced with this choice between mid-level and high-level, what should we choose? I think we should have a consistent approach across the Gloo ecosystem.

I propose that we always expose the high-level APIs (Future, Stream, etc.) and then have a submodule which exposes the mid-level APIs.

That makes it clear that the high-level APIs are the idiomatic APIs that generally should be used, but still makes the mid-level APIs accessible for unusual circumstances.

To start the bikeshedding, I propose we consistently use the name raw for the mid-level submodule.

@David-OConnor
Copy link
Member

David-OConnor commented Mar 18, 2019

We could have dedicated utility crate that wraps common/verbose web_sys features, which the bulk of the modules (I agree they should be high-level) can call. Eg:

/// A convenience function for logging to the web browser's console.  See also
/// the log! macro, which is more flexible.
pub fn log<S: ToString>(text: S) {
    web_sys::console::log_1(&text.to_string().into());
}

/// Convenience function to avoid repeating expect logic.
pub fn window() -> web_sys::Window {
    web_sys::window().expect("Can't find the global Window")
}

/// Convenience function to access the web_sys DOM document.
pub fn document() -> web_sys::Document {
    window().document().expect("Can't find document")
}

/// Simplify getting the value of input elements; required due to the need to cast
/// from general nodes/elements to HTML__Elements.
pub fn input_value(target: &web_sys::EventTarget) -> String {
    if let Some(input) = target.dyn_ref::<web_sys::HtmlInputElement>() {
        return input.value();
    }
    if let Some(input) = target.dyn_ref::<web_sys::HtmlTextAreaElement>() {
        return input.value();
    }
    if let Some(input) = target.dyn_ref::<web_sys::HtmlSelectElement>() {
        return input.value();
    }
    "".into()
}

These could be used either internally to Gloo modules/crates, or exposed in APIs.

@Pauan
Copy link
Contributor Author

Pauan commented Mar 18, 2019

@David-OConnor I had thought of that, but that causes extra maintenance burden for everyone: crate authors and crate users. Putting the APIs as a submodule is much easier for everyone.

I'm not against it, though I think there'd have to be a compelling argument to justify the extra complexity.

If you meant that we should have a single utility crate, I don't think that's a good idea either: it will just end up as a jumbled mess of unrelated APIs.

Keep in mind that even though right now the APIs are small and easy to manage, that won't be true forever: as Gloo becomes larger and more APIs are built, proper maintenance and API layering becomes very crucial.

@fitzgen
Copy link
Member

fitzgen commented Mar 18, 2019

Thanks for starting this discussion, @Pauan, I think this is something we should definitely come to consensus on, and document in CONTRIBUTING.md.

So when we are faced with this choice between mid-level and high-level, what should we choose? I think we should have a consistent approach across the Gloo ecosystem.

+1 for being consistent.

I propose that we always expose the high-level APIs (Future, Stream, etc.) and then have a submodule which exposes the mid-level APIs.

What I care about more than exactly how it is done is exposing all layers of the onion as reusable pieces:

  • -sys bindings (which gloo is not responsible for, since they are in js-sys and web-sys)
  • the direct translation of the JS API into a Rust API (e.g. FnOnce callback-based set_timeout API; I believe this is what you are proposing for raw modules)
  • the version for if-we-were-going-to-design-this-same-abstraction-level-API-in-rust (e.g. using Futures for timeouts and Streams for intervals)

and then optionally, maybe, sometimes:

  • a version of the API at an even higher-level abstraction than what the web exposes (e.g. vdoms, or building couchdb-esque thing on top of indexed db, etc). I think this should have a much higher bar to pass before we include it in Gloo, maybe we wouln't even include these APIs in Gloo since they should probably be experimented with outside Gloo before we canonicalize it within Gloo, and it is a bunch fuzzier in my mind since it is very case-by-case specific by nature.

As far as how to expose the different layers of the onion, I think these are our choices:

  • expose every layer in the root gloo crate. We don't want to do this by design, since we have an explicit goal of spinning things out into reusable crates.
  • Expose every layer for the whatever API in the root of the same gloo-whatever crate. This is what the proof-of-concept gloo-timers crate is currently doing.
  • Expose the layers of the whatever API with some sort of module scheme in the gloo-whatever crate. A variant of this approach is being suggested by @Pauan and the raw module, IIUC.
  • Expose the layers of the whatever API in different crates with some sort of naming scheme, e.g. gloo-whatever-raw, gloo-whatever-futures, etc.

I don't have any strong opinion here, but I'd note that the last has the best compile times for users (e.g. if they only used gloo-whatever-raw and weren't using futures, then they wouldn't need to compile futures) and the highest maintenance overhead for us. Throwing everything into the top-level of a crate has teh least maintenance burden for us, but the coarsest grain for users and would force someone who wasn't using futures to compile the futures crate and the futures+timers integration, for example.

@Pauan
Copy link
Contributor Author

Pauan commented Mar 18, 2019

@fitzgen I generally agree with everything you said.

I don't have any strong opinion here, but I'd note that the last has the best compile times for users (e.g. if they only used gloo-whatever-raw and weren't using futures, then they wouldn't need to compile futures) and the highest maintenance overhead for us.

If we used Cargo features, could we achieve similar fast compile times even with my raw submodule idea?

Throwing everything into the top-level of a crate has teh least maintenance burden for us, but the coarsest grain for users and would force someone who wasn't using futures to compile the futures crate and the futures+timers integration, for example.

Indeed, I think the raw submodule has a relatively nice balance between making things easy for us, and making things easy/fast for end users. I'm not sure how well it will work out in practice, though.

@fitzgen
Copy link
Member

fitzgen commented Mar 18, 2019

If we used Cargo features, could we achieve similar fast compile times even with my raw submodule idea?

Correct. Again, at the cost of maintenance on our end (although probably less than a whole new crate, and neither is that much of a burden anyways...)

@alexcrichton
Copy link
Contributor

One thing I'm curious about is how often we'd be able to distinguish a middle/high layer to abstract here (e.g. how many places it'd make sense to have a raw submodule). The true 'raw' to me is the *-sys crates, and then something like a FnMut vs Stream for setInterval is less clear as to which is high and which is mid. For example the "most optimal" Stream implementation might not actually use the FnMut one but rather the raw web_sys apis.

I suppose this also affects the compile time points you brought up @fitzgen because if the high level apis don't build on mid-level apis, then there's compile wins to be had for sure. If high builds on mid, though, I'm not sure crate separate will make much of a difference.

@kellytk
Copy link

kellytk commented Mar 18, 2019

@fitzgen Regarding

  • a version of the API at an even higher-level abstraction than what the web exposes (e.g. vdoms, or building couchdb-esque thing on top of indexed db, etc)

I think that

maybe we wouln't even include these APIs in Gloo since they should probably be experimented with outside Gloo before we canonicalize it within Gloo

is wise. Identifying those needs and putting out a request-for-implementations to the community for each would likely produce sufficient variety for experimentation while retaining gloo contributor resources for focusing on the core layers.

@fitzgen
Copy link
Member

fitzgen commented Mar 18, 2019

Instead of raw, should we do a context specific name?

E.g. for timers we would have a callbacks module and a futures module?

@Pauan
Copy link
Contributor Author

Pauan commented Mar 19, 2019

@alexcrichton For example the "most optimal" Stream implementation might not actually use the FnMut one but rather the raw web_sys apis.

It's not about internal details, it's about the API that is exposed to the user. From the user's perspective, a callback API is clearly lower level than a Future/Stream based API.

I agree that there might be some gray areas where there isn't a clear division, but in the case of callbacks vs Future/Stream, it seems quite clear to me.

If high builds on mid, though, I'm not sure crate separate will make much of a difference.

That's a good point. Personally, I think the high level APIs generally should be built on top of the mid level APIs, and if they aren't then that indicates the mid level APIs are designed poorly.


@fitzgen E.g. for timers we would have a callbacks module and a futures module?

The downside is that it doesn't make it clear that Futures are the "blessed" way of doing things... but maybe that's actually not a downside?

Maybe Gloo should be more unopinionated and just present both APIs fairly, and let the user decide. So I rather like that idea.

@alexcrichton
Copy link
Contributor

I like the idea of just providing uniform representations of APIs across components, and having a common interface (ish) for both callback and futures-based primitives seems like a good idea. Sharing as many idioms across crates would be great (aka naming and API decision)

@fitzgen
Copy link
Member

fitzgen commented Mar 19, 2019

Personally, I think the high level APIs generally should be built on top of the mid level APIs, and if they aren't then that indicates the mid level APIs are designed poorly.

Strong agree.

@fitzgen
Copy link
Member

fitzgen commented Mar 19, 2019

I think we all seem to be in agreement here except maybe some bike shedding on exact naming of modules? Given that, would you like to make a PR that formalizes this stuff in CONTRIBUTING.md, @Pauan? After we merge that, then we can also update the gloo-timers crate to match the decided upon conventions.

@Pauan
Copy link
Contributor Author

Pauan commented Mar 20, 2019

PR created: #34

I think we should also discuss how deeply we should add in support for Signals.

As I mentioned in some of the other issues, there are some APIs that are a perfect fit for Signals (e.g. DOM is_ready(), getting the current URL for the Router, and many more in the future)

With the proposed system, that is easy to do: we just create a signal submodule, which lives alongside the callback, future, and stream submodules.

@kellytk
Copy link

kellytk commented Mar 20, 2019

I've written 'FRP' style code (using RxJS) and it's phenomenal to develop and maintain. It does have a slight conceptual learning curve however in my experience it's paid off as well as that of Rust itself.

@fitzgen
Copy link
Member

fitzgen commented Mar 22, 2019

Closing this issue for now, since I think we came up with a general plan for how to support different bits of the ecosystem in our crates. I think the question of whether to integrate with a specific crate/ecosystem can move into dedicated issues, for example the FRP/signals discussion should continue in #33

Thanks everyone!

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

5 participants