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

Named reactive declaration #532

Closed
raythurnvoid opened this issue Sep 6, 2020 · 8 comments
Closed

Named reactive declaration #532

raythurnvoid opened this issue Sep 6, 2020 · 8 comments
Labels
feature request New feature or request Fixed Fixed in master branch. Pending production release.

Comments

@raythurnvoid
Copy link

Having multiple reactive declarations makes difficult to understand what each reactive declaration actually do.

Having the possibility to name a reactive declaration helps with code readability since it gives the declaration some context of what it does.
Moreover vscode outline could be synched with the given name.

image

@dummdidumm dummdidumm transferred this issue from sveltejs/language-tools Sep 6, 2020
@dummdidumm
Copy link
Member

Named labels is something Svelte itself needs to support before we can add functionality like this to the language tools. For that reason I transfered the issue to the Svelte repo.

@Conduitry
Copy link
Member

What would the compiler need to do here? Treat these comments specially and put the name in some new field in the AST? The compiler doesn't feel like the right place for that special handling to me.

@dummdidumm
Copy link
Member

I understood it as if the request is about allowing anamedlabel$: ... but looking closely at the screenshot there's also this @name comment. I don't know what exactly is the request then. @monkeythedev could you please clarify?

@pngwn
Copy link
Member

pngwn commented Sep 6, 2020

I think the request is for the outline section on the left to pick up the tsdoc comment and apply the provided name as the function name. I think this is probably a language-tooling request?

@Conduitry
Copy link
Member

That's what I also thought, and I agree that this sounds like a tooling-only feature request.

@raythurnvoid
Copy link
Author

raythurnvoid commented Sep 7, 2020

@dummdidumm @Conduitry I used a comment just as an example, another way to name it through the compiler could be:
reactOnMyThing$: ... reactive code ... as @dummdidumm wrote.
In this way the comment is no more needed and also the call stack would show the name.

@Conduitry
Copy link
Member

Treating foo$: as a reactive declaration/block would technically be a breaking change anyway. I'm going to move this back over to the language tools repo for consideration of the comment mechanism.

@Conduitry Conduitry transferred this issue from sveltejs/svelte Sep 10, 2020
@dummdidumm
Copy link
Member

dummdidumm commented Sep 13, 2020

I played around with the outline view a little. To be consistent with how TS/JS does it, I'm against the comment solution. So instead I propose to show the first 40-50 characters of that reactive statement, which should give enough info at a glance in most situations.

dummdidumm pushed a commit to dummdidumm/language-tools that referenced this issue Sep 13, 2020
sveltejs#532
- remove svelte2tsx transformation artifacts within script
- show first 50 characters of reactive statements that are not variable assignments
@dummdidumm dummdidumm added the feature request New feature or request label Sep 13, 2020
dummdidumm added a commit that referenced this issue Sep 14, 2020
#532
- remove svelte2tsx transformation artifacts within script
- show first 50 characters of reactive statements that are not variable assignments
@dummdidumm dummdidumm added the Fixed Fixed in master branch. Pending production release. label Sep 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request Fixed Fixed in master branch. Pending production release.
Projects
None yet
Development

No branches or pull requests

4 participants