Skip to content

Conversation

augustjk
Copy link
Member

No description provided.

Copy link

A live preview of this PR will be available at the URL(s) below.
The latest URL will be appended to this comment on each push.
Each build takes ~5-10 minutes, and will 404 until finished.

https://pr1277-c495eec---lit-dev-5ftespv5na-uc.a.run.app/

@@ -369,7 +369,7 @@ The `ValueType` type parameter is the type of value that can be provided by this

### `@provide()`

A property decorator that adds a ContextConsumer controller to the component which will try and retrieve a value for the property via the Context API.
A property decorator that adds a ContextProvider controller to the component making it respond to any `context-request` events from its children consumer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: thoughts about children consumer -> child consumers?

Copy link
Member Author

Choose a reason for hiding this comment

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

I took it straight from the JSDoc here
https://github.com/lit/lit/blob/f60a3a2c994f41fc3df1bd8a76451ea185b66e11/packages/context/src/lib/decorators/provide.ts#L19-L20

While I don't oppose to the new wording itself, I think it's minor enough that I'd rather keep it in sync with what's on the source code. Thanks for the suggestion though!

Copy link
Contributor

Choose a reason for hiding this comment

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

Great call!

Copy link
Contributor

@AndrewJakubowicz AndrewJakubowicz left a comment

Choose a reason for hiding this comment

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

My comment is non blocking because this is a pretty glaring error you have fixed. Thank you!

@augustjk augustjk merged commit 9587499 into main Nov 29, 2023
@augustjk augustjk deleted the context-doc-fix branch November 29, 2023 02:23
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.

2 participants