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

docs: clarify let: directives #8495

Closed
wants to merge 4 commits into from

Conversation

jamesscottbrown
Copy link
Contributor

@jamesscottbrown jamesscottbrown commented Apr 12, 2023

This tries to clarify the explanation of let: directives through 3 changes:

  • use a name other than prop for the specific prop in the example. Using the generic name prop might make a reader wonder if this is a keyword or magic name choice that needs to be used
  • add some comments to the example
  • re-phrases the explanation to avoid the reader having to think about which direction is backwards

I found the statement "Slots can be rendered zero or more times and can pass values back to the parent" initially unclear because there are two dfferent child-parent relationships to consider (a component definition with a child <slot>, and a use of the component with a child slot template).

I think these changes are helpful because this syntax is inherently somewhat confusing: as noted in #4125, let:foo={bar} defines bar to be a new variable set to the value of foo, which is counter-intuitive.

@vercel
Copy link

vercel bot commented Apr 12, 2023

@jamesscottbrown is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

@jamesscottbrown jamesscottbrown changed the title clarify let: directives docs: clarify let: directives Apr 12, 2023

The `let:` directive gives a slot template access to variables defined in the component that they are provided to.
Copy link
Member

Choose a reason for hiding this comment

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

I find this sentence much harder to read/parse than the previous one. Can we find some kind of middle ground between what was there before and what's proposed?

Copy link
Member

@ghostdevv ghostdevv May 11, 2023

Choose a reason for hiding this comment

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

Suggested change
The `let:` directive gives a slot template access to variables defined in the component that they are provided to.
The `let:` directive gives a slot template access to variables passed down by the parent component.

@PuruVJ
Copy link
Collaborator

PuruVJ commented Jul 17, 2023

Hi! Could you resolve merge conflicts? The file structure has changed, so you may wanna move these changes to documentation/docs/02-template-syntax/07-special-elements.md 😁

@benmccann
Copy link
Member

Hi @jamesscottbrown. Are you still interested in pursuing this PR? If so, the merge conflicts will need to be addressed

Copy link

changeset-bot bot commented Dec 4, 2024

⚠️ No Changeset found

Latest commit: c360a6f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Rich-Harris
Copy link
Member

Thank you. I agree with your points, though I also agree with the comments saying that the proposed change makes things less clear. I've opened #14543 which updates the current docs with changes inspired by these ones; since this PR is stale I'll close it

@Rich-Harris Rich-Harris closed this Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants