skeletonlabs
Don't add data-testid attributes to component roots
Describe the feature in detail (code, mocks, or screenshots encouraged)
Skeleton includes data-testid attributes in the root of each component. It would be great if those were not included in the distributed library.
What type of pull request would this be?
Enhancement
Provide relevant links or additional information.
Why: I work on a team with a dedicated test engineer. He assumes that data-testids are stable, and won't be changed without warning. It would be confusing to explain that some data-testids are not under our control and should not be relied on, especially since there's nothing obviously marking these data-testids as belonging to skeleton.
Feel free to change this to a bug; wasn't sure if bug or feature request was more appropriate for this.
Building on my reply here: https://github.com/skeletonlabs/skeleton/discussions/3499#discussioncomment-12785069
We'll consider this, but we definitely need to be able to test our own components before we ship them. It may be possible to strip them, but my counter-proposal is to supply prefixes for these instead. And if possible, allow folks to override them if they prefer. Perhaps with the component API change this will be more likely.
@Hugos68 I'd appreciate your thoughts on this as well.
With the proposed component API, you could easily set the testid in the test, instead baking it into the component itself.
Prior to that, a skeleton prefix would be appreciated
edit: rereading this comment, it could maybe sound demanding. I hope it didn't come across that way. I really appreciate your work 🙏
Building on my reply here: https://github.com/skeletonlabs/skeleton/discussions/3499#discussioncomment-12785069
We'll consider this, but we definitely need to be able to test our own components before we ship them. It may be possible to strip them, but my counter-proposal is to supply prefixes for these instead. And if possible, allow folks to override them if they prefer. Perhaps with the component API change this will be more likely.
@Hugos68 I'd appreciate your thoughts on this as well.
With our new component API we don't need to put data-testid on the components because we can pass them as a prop in the test suite
Just occured to me @endigo9740 if the collapse props proposal goes through, what is there left to test? ZagJS will handle testing logic. No more Xclasses or anything. So what do we want to test at that point?
I still think we should reserve the right to augment the templates and features within reason. Not a total conversion. But if we for example want to replace a hardcoded icon with a slot/snippet/reactNode then that seems reasonable to test.
Why can't you provide a property to override the data-testid that you are providing? This should be a default property on all the components that users interact with. For example, controlDataTestid
We have recently upgraded from v2, and used data-testid, but now can not set to our expected stable values.
Why can't you provide a property to override the data-testid that you are providing? This should be a default property on all the components that users interact with. For example, controlDataTestid
We have recently upgraded from v2, and used data-testid, but now can not set to our expected stable values.
Honest answer: just an oversight. We didn't often had use cases that people wanted to use our components for their testing use cases as you can wrap containers with display contents.
In the new Component API proposal this is solved. But we haven't found much time lately to work on it.
Starting in the upcoming Skeleton v4 release you will be able to pass any arbitrary attribute to the component template, this includes data-test attributes. Which are no longer provided per each component element by default. Keep out eye out for this release soon (tm)
Thanks. Though we just went through the pain of migrating from v2 to v3, we may not migrate to v4, depending on how much changes.
@adamdva I wouldn't compare the scope of v3 and v4.
The v3 update included:
- A top-to-bottom rewrite of Skeleton
- Introduction of Zag-based components; including multi-framework support
- Updates to support Svelte 5
- Updates to Support Tailwind 4
(there was no way this wasn't going to have some level of pain)
The v4 update:
- Minor CSS import change (you can drop
@source) - Updates to the component tree structure, as described here.
- The introduction of ~3-4 brand new components
We're aiming to automate as much of the migration as well, but there may still be some manual adjustments required due to nuance in each project and how folks consume components.
I know these changes can be painful, but they're necessary so we can finally solidify our APIs and begin supporting 4 total frameworks (Svelte, React, Vue, and Solid.js). So I appreciate everyone's effort in this journey, but the bumpy part is almost over.