-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
@slot and @event description bugs #102
Comments
Note I've just noticed Issue 2 may be intended behavior per your README instructions:
We would love to be able to add descriptions to forwarded events, but I'd understand if it's not feasible right now. So that just leave issue 1 |
Hi Chris, thank you for reporting this, along with the detailed screenshots. Come to think of it, I don't think there's any reason why The same goes for default slots not allowing a description. |
@endigo9740 For the first issue, could you try adding an explicit type annotation for the default slot? - @slot
+ @slot {{}} I'm testing this with the Sveld GUI (https://sveld.onrender.com/), and the |
@metonym That would be awesome if we could handle both scenarios. For our project we use the default slot for different reasons, so being able to specify the intention would be great. In a lot of cases it's a set of child elements, like a set of Tabs for a Tab Group. I think most of our events will be forwarded default Svelte events (on:click, etc), so we can try and communicate that, but having it be explicit with the description would be great. I'm away from my workstation right now so I'll ask another contributor to test your suggestion above and we'll report back shortly. |
@metonym FYI I noted you're using the multi-line comment format, whereas I was using single line, so I've corrected that on my end. That was the source of some of my troubles, so that's my bad! As for your suggestions above - comparing notes with @niktek, our contributor, we're seeing some strange results. In the "onRender" playground we can replicate the results you shown above 1:1, but locally in our project results do not match. In the playground: In our project: It's like it's killing all the descriptions for some reason. I'm not sure if it's relevant, but FYI we're utilizing Sveld via vite-plugin-sveld v1.1.0. Perhaps this is influencing things? We originally tried setting up Sveld as a Rollup plugin in our SveldKit project, but weren't able to get this working, so we opted for the plugin. If you have any guidance that could help we could try that again. Perhaps eliminate one point of failure here? |
Given that |
FYI I've been testing a few combination of comment formats. Here's what does or doesn't work: Multiple single-line comments with an empty slot as the first definition - this works: Multiple single-line comments but replacing the empty slot with just any regular Multi-line comment format - this does not work: I've tried various versions of the multi-line comment, but none of these work. |
I am surprised that the last example does not work. Given the input: <script>
/**
* @slot {{}} content - This is the CONTENT slot description.
* @slot {{}} lead - This is the LEAD slot description.
* @slot {{}} summary - This is the SUMMARY slot description.
*/
/**
* @event {{}} click - This is the CLICK event description.
*/
</script> I see this in the JSON output: I don't think it's a problem with |
I deleted the VIte cache per your instruction and interestingly enough your example above doesn't work, but when I keep the leading // Slots:
/**
* @slot {{}} content - Allows for an optional leading element, such as an icon.
* @slot {{}} lead - Provide the summary details of each item.
* @slot {{}} summary - Provide the content details of each item.
*/ Removing that leading comment makes all the descriptions disappear. |
FYI If you want to replicate what we're seeing, you can. Skeleton is open source, so you can pull down and run a version locally if you wish. This is a standard SvelteKit project. EDIT The component I'm testing this on is located in
Just make sure to tap the Slots tab. Also we've noted the HMR doesn't work with Sveld, so a manual page refresh is needed between code changes for anything Sveld-related. I'm happy to walk you through how we're passing the data around if need be. Otherwise here's a direct link to the file on GitHub if you want to just do a quick scan and see if anything jumps out at you: |
Hey, my name is Chris and I'm the project lead and core contributor to Skeleton:
https://skeleton.dev/
We're in the process of integrating Sveld to automatically document our various components in our library. Unfortunately we're running into a couple issues with the new @slot and @event description fields introduced here:
Here's how we're defining our JSDoc comments for a component. This includes 3x Slots and 1x forwarded Event:
Issue 1 - First Slot Always Dropped
Per our JSDoc definitions above you'll see we're adding an empty
@slot
definition, which should come out to a total of 4x slots. But we've noted that regardless of how we structure or order the comments, the first @slot definition is always pruned. The only work around we've discovered is to insert an empty slot to be "sacrificed".Issue 2 - @event changes types
We've applied a single
on:click
event to our template. This should be forwarded up. Here's what that looks like if we do not specify a JSDocs comment, just allow Sveld to auto-document it:The above is correct, though missing the description obviously since one isn't specified.
However, when we do specify a description, we see two issues:
type
information incorrectly changes fromforwarded
->dispatched
element
value - though I believe this is due to the fact it's being treated as "dispatched"FYI we're still testing, so we might update this post or create another if we discover other issues. However, these are the two most pressing we've encountered.
The text was updated successfully, but these errors were encountered: