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

Add context API #975

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kevinkucharczyk
Copy link

@kevinkucharczyk kevinkucharczyk commented Sep 28, 2023

This is an RFC to add a context API to Ember, based on #775 (and various past requests in Discord)

Preview here

@github-actions github-actions bot added the S-Proposed In the Proposed Stage label Sep 28, 2023
matter how deeply nested they are inside the card, without us having to
explicitly pass the background color through multiple layers of components.

- Chart and graphical components
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another use case is drastically simplifying and improving ember-kepboard, for having real hierarchical shortcuts

This introduces a new class, which keeps track of "context provider" components,
and exposes their state to any descendant components.

The final implementation of context in Ember might be by using special
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way we could avoid string keys?

What would this look like in a template-only component?

Or gjs/gts?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question! Template-only components are definitely an interesting use case to consider. One approach might be to also ship provider and consumer components, which could be invoked in any template, like:

<ContextProvider @key="my-context-name" @value={{this.myValue}}>
</ContextProvider>
<ContextConsumer @key="my-context-name" as |val|>
 {{val}}
</ContextConsumer>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a primitive, which could be used for building decorators, template helpers, and provider components should be added first. Maybe a public API which allow to detect if something is rendered as a child of something else, could enable experiments in userspace?

Maybe two functions low-level functions are all which is needed:

getRenderPosition(): Symbol;
isChildOf(potentialChild: Symbol, parent: Symbol): boolean;

This could be extended to isParentOf, isSiblingOf, and isDescentOf.

The functions could only be used within a render cycle. But component, helper, and modifier installation as well as updates happens during render cycle. So that should be okay.

The functions should throw if being called outside of a rendering cycle.

By only exposing a symbol rather than a full rendering tree, we avoid leaking internals of the Glimmer VM.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My general take is this should pretty intentionally mirror the service API, and if the way we declare services changes then the way we declare contexts also changes, but barring that the syntax is roughly identical

@oriSomething
Copy link

I actually wanted to write an RFC for this in the past, but I it didn't get to a point I was happy enough with what I thought.

Before I give some of my points, More reasons for having context, and especially the keyboard management, that I've used similar technic at previous work, and it had a really good DX (and also I found out, Meta does something similar).

A few notes:

  • I think the the context in the end should be some class that you instantiate, main reasons:
    • The shape of the object (we don't want a mess around the tree of polymorphic shapes)
    • The ability to instantiate a root context value lazily when needed, as service. (You can think of it as "contextual service")
  • Issues I had is in cases where a component got the context value, but also provided it:
class {
  @context(Whatever) contextValue;
  @context.provide(Whatever) provideContextValue() { // <- Making bad name
    // Should I access upstream context by `this.contextValue` or having Api as `useContext(…)`?
    // Isn't it bad we have multiple properties for the same context value, and worse, multiple providers?
    // What is the right way to instantiate a new context instance? is `new Whatever()` is good enough
    // What do I do to differentiate the root context value from the "downstream" context values?
  }
}
  • In a way, React has the "correct" API, but it's also a painful with current Ember's state of components. Maybe with GJS it would be less painful. But I don't think class components should be abounded, or have 2nd class API
  • I agree with @NullVoxPopuli that string keys are issues. And I also think string keys aren't scalable, and also harder to type

@allthesignals
Copy link

allthesignals commented Sep 29, 2023

I implemented something template-only-friendly and similar to this using existing APIs, but it's kind of janky:

https://github.com/allthesignals/ember-rfc-context/blob/main/app/templates/application.hbs#L2-L11

  <Context::Provider @name='design' @value={{this.value}} as |value|>
    <Card @backgroundColor={{value}}>
      <Button />
    </Card>

    <Context @name='design' as |value|>
      hi {{value}}
    </Context>

  </Context::Provider>

Descendants:
https://github.com/allthesignals/ember-rfc-context/blob/main/app/components/button.gjs#L6-L8

    <Context @name='design' as |value|>
      {{value}}
    </Context>

Looks similar to the React API 🤷

@simonihmig
Copy link
Contributor

Some loose thoughts...

  1. another use case is managing hierarchies in DOM-less worlds (e.g. 3D scene graph), see some previous discussions here: A case for DOM-less rendering #597 (comment)
  2. I agree we need 1st class support for TO/gjs.
  3. Creating context within the template akin to the example given by @allthesignals would allow that. But also it seems the current draft would imply that a component can only create a single context (per key), while creating the context within the template would allow for spawning multiple contexts (even with the same key) for different parts of the component's sub tree.
  4. Context is a value, has a lifetime and would probably benefit from lazy evaluation. Therefore, does it make sense to unify that concepts with resources?
  5. I have been thinking a bit about the future role of Dependency Injection in the new Ember world. Which I think is going to largely focus only on services, given that with strict mode templates we are moving a lot of things out of the path of DI and resolver logic. And given that injected services are basically app-level dependencies (resources!?), would it make sense to rethink and generalize the concept of DI to include app-level as well as render-tree level "dependencies", with the latter basically being what we are calling "context" here? I fell it kinda makes sense at least when looking at Vue's nomenclature of provide and inject.

Just thinking aloud...

@kevinkucharczyk
Copy link
Author

kevinkucharczyk commented Oct 1, 2023

We've just open sourced some of our internal recent experiments with an Ember context API, this can be found at: https://github.com/customerio/ember-provide-consume-context. Hopefully seeing that version of a context API in action helps spark some more ideas on this topic!

To continue the discussion on some of the points made above:

Instantiating contexts

I think the the context in the end should be some class that you instantiate

Personally I'm actually not opposed to this! I've been working with React a little lately, and I find createContext makes a lot of sense. I even started with that sort of API in our context experiments (see this state of our repo).

You also point out the one downside we found with this approach: importing the context objects into templates was a bit annoying. But yes, in a GJS world this wouldn't be an issue at all, so perhaps this would be the way to go for the official context API.

String context keys

I agree, string keys can be hard to manage, and could potentially not scale well. Scaling and code maintenance could be improved by making sure to always declare string constants and export/import those rather than using the strings as-is in templates, but that comes with the same pain-points as the createContext-like approach mentioned above. So if we're recommending exported string constants, we might as well go for a create context function.

Type-safety is definitely a concern. A createContext function is great for this, we can type it with generics and feed it whatever value types we expect from our context.

In the current state of our repo, we added a "type registry" for the string context keys, where the string keys can be mapped to context value types, much like components can be added to the Glint template registry.

I do agree that this isn't the most ergonomic approach though, and I'm not attached to using strings for an official API!

Creating context within the template akin to the example given by @allthesignals would allow that. But also it seems the current draft would imply that a component can only create a single context (per key), while creating the context within the template would allow for spawning multiple contexts (even with the same key) for different parts of the component's sub tree.

The @provide decorator approach indeed only allows creating one context for a particular key in one component. However, in our addon, we also include ContextProvider and ContextConsumer components, which can be used to provide multiple contexts of the same key in one template, or provide/consume contexts in template-only components. For example:

<ContextProvider @key="my-context-name" @value={{this.firstContextValue}}>
  // Nested component tree
</ContextProvider>

<ContextProvider @key="my-context-name" @value={{this.secondContextValue}}>
  // Another component tree
</ContextProvider>

Whether those sort of components make it into the final API, I'm not sure. But this should serve as a good example of what can be done.

Dependency injection

I fell it kinda makes sense at least when looking at Vue's nomenclature of provide and inject.

It's actually interesting to look at the source of how Vue and Svelte handle context:

inject is maybe a bit of a misnomer. Both implementations basically just pass context objects down the component tree, and "injecting" is simply reading from the object.

I agree though that in the Ember world, where the concept of dependency injection is actively used and an important part of how many elements of the system work, a context API might be better implemented as another part of this DI setup, or via some resource-based approach you suggest.

@runspired
Copy link
Contributor

The more I think on context the more I want to build it into Ember. After thinking and poking around a few of the implementations out there, I think the dom-tree scoped approach is both efficient and the most versatile: https://github.com/customerio/ember-provide-consume-context

Other approaches try to improve the DX (avoid the nested provider right-drift syndrome) but sacrifice by either making contexts universal to all components in a route or by key to all components in the app. This sacrifice is quite large if you want to say route multiple things on a page at once, or migrate only part of a page to using new patterns and need to isolate some concern for it.

Would love to advance this as part of the polaris experience, especially because I see contexts as key part of the path for folks rewriting from ModelFragments to modern EmberData.

@kevinkucharczyk
Copy link
Author

@runspired I'm really glad to see there is support for this proposal!

Please let me know if there's anything I can do to help move it along, I'd be more than happy to keep working on this.

@runspired runspired self-assigned this Mar 15, 2024
@runspired
Copy link
Contributor

On @oriSomething's feedback, I feel the opposite about a few points:

think the the context in the end should be some class that you instantiate, main reasons:
The shape of the object (we don't want a mess around the tree of polymorphic shapes)
The ability to instantiate a root context value lazily when needed, as service. (You can think of it as "contextual service")

For starters, I think it should be valid to provide an already instantiated thing as the context. This allows regionalizing portions of an app during migrations (or for data isolation concerns). This even goes so far as enabling context<->service crossover. The specific motivating example I have is that I would like to have two instances of EmberData's store active simultaneously, with different configurations and separate caches, such that an application can migrate from one set of behaviors to another by region of an app safely.

Further, services at least have an easy time with DI gaining access to additional configuration etc. A context API that takes a class token would struggle to allow this.

Lastly, it is almost certainly the case that contexts will be consumable by components provided by external libraries. If the keys are anything other than strings this results in nearly impossible to resolve cycles and unpublishable typescript setups. We've already quickly hit the limitations of non-string-keys while exploring services and the more we encourage monorepo patterns the more often those limitations will get hit.

@ef4 ef4 added S-Exploring In the Exploring RFC Stage and removed S-Proposed In the Proposed Stage labels Mar 15, 2024
@ef4
Copy link
Contributor

ef4 commented Mar 15, 2024

Discussed this at RFC review. General support for continuing to explore this. Feedback:

  • teaching story will need to explain when to use this and when not to (easy for app authors to overuse and get spaghetti, powerful feature good for library/framework authors)
  • @jelhan's comment about offering a low-level primitive instead is worth investigating. Some primitives like that are already needed by ember inspector and maybe could unify.
  • nice test utilities would be needed for providing context easily to tests and groups of tests.
  • use cases that are important:
    • ember-engines next generation could be context based
    • ember-data team has expressed intrest in using context to aid migration and implement store forking

@ef4
Copy link
Contributor

ef4 commented Apr 26, 2024

RFC review meeting reviewed this and there is strong interest and support. This can't really advance until it has a more complete How We Teach This and the previously-discussed issues are addressed.

@kevinkucharczyk
Copy link
Author

One of the things we'll need to reach a consensus on is how context is provided/injected in the first place.
That is, do we use the proposed decorators and string keys (we could also support symbol keys), or should the API be more like React's createContext, where a context object is instantiated and passed around to use as a reference.

Personally, I'm still in favour of the string keys and decorators approach. It's basically identical to how we use services, and is a pattern Ember developers are familiar with.

@ef4 what would be the best way to keep having these discussions so we can reach a decision? Maybe the upcoming EmberConf is our opportunity to talk about context.

In the meantime, I've updated the proposal with a "How We Teach This", and some notes on testing.

@jelhan
Copy link
Contributor

jelhan commented May 1, 2024

One of the things we'll need to reach a consensus on is how context is provided/injected in the first place.

I don't think we need a consensus here yet. Instead we should add low-level primitives unlocking experiments with the different approaches in user space. Following the well established and successful change management processes we used for modifiers, template authoring format and many more new features in the last years.

  1. Unlock experiments by adding required low-level primitives
  2. Let community experiment, gather experience and settle on a high-level API
  3. Add support for the high-level API in Ember itself

I don't see any reason why we should do it here differently. Especially as the low-level primitives may unlock experiments we cannot even foresee now.

@provide('form-context')
get formState() {
return {
model: this.args.model,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nicer to split this out into something like this to reduce ambiguity a little bit (model here isn't an ember data model):

Suggested change
model: this.args.model,
values: this.args.values,
validations: this.args.validations,
errors: this.errors,

Then below, you could have an get errors() {} property which runs the validations on the data and returns an object.

Mostly I think it would be good to make this demo component a little richer to avoid any issues of, "the parent component could have just provided the form context, why does this component need it?", etc.

@josemarluedke
Copy link
Sponsor

should the API be more like React's createContext, where a context object is instantiated and passed around to use as a reference.

This approach is probably better in terms of supporting TypeScript users in a nicer way. Using string keys seems a step backward, as folks are also experimenting with injecting services using references instead of strings. (see https://github.com/chancancode/ember-polaris-service).

@runspired runspired added T-ember-data RFCs that impact the ember-data library T-framework RFCs that impact the ember.js library T-learning labels May 17, 2024
@runspired
Copy link
Contributor

We've discussed this in a few RFC meetings now and generally would like to continue to advance this, but with two points of feedback:

  • This RFC should explain that in-effect, services are just context keys on the broadest possible scope (app-wide).
  • the API for services (@service('name') key; and the API for contexts (@consume('name') key;) should be identical. If we decide later to change from string keys to another form of key, we will take on both APIs simultaneously, but it will be better for this RFC to not try to solve that problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-Exploring In the Exploring RFC Stage T-ember-data RFCs that impact the ember-data library T-framework RFCs that impact the ember.js library T-learning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants