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 default prop values, attribute behavior and text expressions #15257

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

melihguleyupoglu
Copy link

@melihguleyupoglu melihguleyupoglu commented Feb 10, 2025

What does this PR do?

  • Clarifies the behavior of default prop values.
  • Explains how null and undefined affect attributes and text expressions in markup.

Issue Reference

Closes #14716

Additional Information

  • Tests have passed successfully ✅
  • Linting have passed successfully ✅

Copy link

changeset-bot bot commented Feb 10, 2025

⚠️ No Changeset found

Latest commit: cf30a6e

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.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

@svelte-docs-bot
Copy link

Comment on lines 46 to 47
<Nested /> <!-- answer is set to default value -->
<Nested answer={undefined}/> <!-- answer is set to default value -->
Copy link

Choose a reason for hiding this comment

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

Maybe it's better to write exactly what value in both lines, for example, "answer is set to 3 (default value)"

<Nested answer={42} /> <!-- answer is set to 42 -->
<Nested answer={null} /> <!-- answer is set to null (default value is not used)-->
<Nested /> <!-- answer is set to default value -->
<Nested answer={undefined}/> <!-- answer is set to default value -->
Copy link

Choose a reason for hiding this comment

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

Add an example with deep nesting

<p>{num}</p> <!-- Renders as: <p>1</p> -->
<p>{bool}</p> <!-- Renders as: <p>false</p> -->
<p>{obj}</p> <!-- Renders as: <p>[object Object]</p> -->
<p>{empty}</p> <!-- Renders as: <p></p> (empty string) -->
Copy link

Choose a reason for hiding this comment

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

Add an example with an empty string, null, and an object with the toString method.
Perhaps it's better to shorten the example by removing the script tag.

@AlexRMU
Copy link

AlexRMU commented Feb 10, 2025

It would also be nice to write somewhere that the expressions in the markup seem to wrap in $derived. Accordingly, expressions with runes will be reactive and the rest will not.


```

Deep nesting refers to having components nested within other components across multiple levels. In this setup, props can be passed from parent to child components through several layers.
Copy link

Choose a reason for hiding this comment

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

I'm sorry, I didn't make myself clear. I meant deep destructuring assignment, but it turns out that it is forbidden.

Copy link
Author

Choose a reason for hiding this comment

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

Should i remove the example?

Copy link

Choose a reason for hiding this comment

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

Yes, I think it's unnecessary here.

let num = 1;
let bool = false;
let obj = { key: "value" };
let objToStr = obj.toString();
Copy link

Choose a reason for hiding this comment

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

I meant this:

<script lang="ts">
    let obj1 = { toString: () => "str" };
</script>

<p>{obj1}</p> <!-- Renders as: <p>str</p> -->

And add a link to MDN.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, ok. My bad. So you want me to override the toString function right?

Copy link

Choose a reason for hiding this comment

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

Add this instead of objToStr.

Copy link
Author

Choose a reason for hiding this comment

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

Should i add the MDN link to documents? Or you just share with me for the reference?

Copy link

Choose a reason for hiding this comment

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

I don't know.

Copy link

Choose a reason for hiding this comment

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

Now let's wait for what the team says.

@melihguleyupoglu
Copy link
Author

Will do, thanks!

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.

Improvement for documentation
2 participants