-
Notifications
You must be signed in to change notification settings - Fork 727
docs(svelte): remove using component warning from rendered results #299
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
Conversation
| Result | Description | | ||
| ------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| `container` | The HTML element the component is mounted into. | | ||
| `component` | The newly created Svelte component. This is mostly useful for testing exported functions or cases where manipulating the DOM doesn't fit. Outside of said cases avoid using the component directly to build tests, instead of interacting with the rendered Svelte component, work with the DOM . | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain such a scenario. This seems to go against the core philosophy of Testing Library and I want to make sure anyone reading this doesn't get the wrong idea that they should be testing implementation details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This update was in reference to this issue.
So before I had touched the svelte-testing-library or the docs the component was already being returned and the docs had no warning at all. My first update couldn't remove the component (backwards compatibility), and so I left a warning comment in the source code, and I wrote a warning (I recently removed) in the new docs.
However, in Svelte you can export functions from a component that can be used on the rendered instance.
Example:
<script>
// Component.svelte
export function myFunction() {
// Performs some operation...
}
</script>
// Test.js
const { component } = render(Component)
// This is the only way to call this method.
component.myFunction()
// You might export a "static" function and use it like this.
component.prototype.myFunction()
You can think of these functions as equivalent to a method OR static method in a React component, with the main difference being that this method is exposed as part of the client API. In React you don't compile components that can be used as vanilla JS components.
In the end I thought there may be edge cases where people need to access these exported functions so we should just return out of convenience for those situations. Otherwise people would need to initialize their components without the render
function and their tests would start getting messy. It'd be best to just rely on the testing library and not duplicate code.
I thought my explanation in the docs was okay because I described the use case and I still mentioned, "Outside of said cases avoid using the component directly to build tests, instead of interacting with the rendered Svelte component, work with the DOM".
Love to hear any suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a comment about some of my thoughts on the specific situation here: testing-library/svelte-testing-library#57 (comment)
If I understand you correctly, you're saying that it's common in svelte for a parent component to access and call directly the methods of a child component? If that's the case, then it makes sense to expose the component in this way (the developer-user is one of the users we need to write tests for after all). But if that's not correct, then we need to make sure that people's tests resemble the way their software is used and if the only place they're accessing component methods is in their tests, then that's problematic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I completely agree with everything you've said, including your comments on the issue. Unit tests like the third test in the issue are brittle and don't add much value, the fourth test I also mentioned can be rewritten to be interacted with through the DOM, just like a user would.
I may have to rewrite the docs to better explain when it is acceptable. Due to Svelte being a compiler and not a framework, many people not only create user facing applications/components but they also compile vanilla js components that have developer facing API's.
Example
Let's say I was creating a video player component with svelte to be distributed and used in other projects, and I had a play function I wanted to be made available on the compiled JS component, so devs of my player can control the video player programmatically and call play
when they want to.
<script>
// VideoPlayer.svelte
export function play() {
}
</script>
What developers would then be able to do with the compiled code is
const videoPlayer = new VideoPlayer()
// Call this method to play the video
videoPlayer.play()
In a testing environment if you wanted to test whether the play button did what you expected without using the component directly, you'd need to create a fixture (wrapper component, add a button that when pressed calls videoPlayer.play()
) and see if the video is playing. All of that would be useless boilerplate and not actually represent how the component is being used.
So this is the use case I was alluding to. I only see the component useful for testing the developer facing API's of svelte components. I thought it'd be a convenient export for those situations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. I agree with your conclusions. It just needs to be really clear when it's useful and when it's not. Maybe linking to https://kcd.im/test-user would help.
No description provided.