Skip to content

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 14, 2019
Merged

docs(svelte): remove using component warning from rendered results #299

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e91fb71
docs(svelte): remove warning on using component from rendered results…
mihar-22 Oct 14, 2019
01b0a5f
docs(svelte): debug description mentions wrong element
mihar-22 Oct 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions docs/svelte-testing-library/api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ Please refer to the

### Results

| Result | Description |
| ------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `container` | The HTML element the component is mounted into. |
| `component` | The newly created Svelte component. **Please do not use this, it will most likely be removed in a future release.** Using this method goes against the testing libraries guiding principles. |
| `debug` | Logs the baseElement using [prettyDom](https://testing-library.com/docs/dom-testing-library/api-helpers#prettydom). |
| `unmount` | Unmounts the component from the `target` by calling `component.$destroy()`. |
| `rerender` | Calls render again destroying the old component, and mounting the new component on the original `target` with any new options passed in. |
| `...queries` | Returns all [query functions](https://testing-library.com/docs/dom-testing-library/api-queries) that are binded to the `container`. If you pass in `query` arguments than this will be those, otherwise all. |
| 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 . |
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

@mihar-22 mihar-22 Oct 16, 2019 •
edited
Loading

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.

Copy link
Member

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.

| `debug` | Logs the `container` using [prettyDom](https://testing-library.com/docs/dom-testing-library/api-helpers#prettydom). |
| `unmount` | Unmounts the component from the `target` by calling `component.$destroy()`. |
| `rerender` | Calls render again destroying the old component, and mounting the new component on the original `target` with any new options passed in. |
| `...queries` | Returns all [query functions](https://testing-library.com/docs/dom-testing-library/api-queries) that are binded to the `container`. If you pass in `query` arguments than this will be those, otherwise all. |

## `cleanup`

Expand Down