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

Support Svelte's $$Generic type #306

Closed
ptrxyz opened this issue Mar 22, 2023 · 21 comments · Fixed by #477
Closed

Support Svelte's $$Generic type #306

ptrxyz opened this issue Mar 22, 2023 · 21 comments · Fixed by #477
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@ptrxyz
Copy link

ptrxyz commented Mar 22, 2023

I'm not sure if this is a bug or a feature but I think svelte-eslint-parser doesn't support Svelte's $$Generic type to type component properties.

image

Relevant information is probably this: sveltejs/eslint-plugin-svelte3#127

@ota-meshi
Copy link
Member

Thank you for posting issue.

The RFC about $$Generic don't seem to have been merged yet.
sveltejs/rfcs#38
I don't want to fully support it yet to avoid a lot of breaking changes.
If you want to support it, we can add it as an experimental feature.

--

Related to sveltejs/eslint-plugin-svelte#390

@ota-meshi ota-meshi added help wanted Extra attention is needed enhancement New feature or request labels Mar 22, 2023
@ptrxyz
Copy link
Author

ptrxyz commented Mar 23, 2023

I see, so it is on the radar somehow but needs some deeper digging. Thanks a bunch.
Feel free to close this issue or keep it open to keep track of $$Generic.

For anyone stumbling across this: for now I simply workaround by defining $$Generic globally in my eslintrc:

...
        env: {
                ...
        }, 
        globals: {
		$$Generic: 'readonly'
	},
...

@dummdidumm
Copy link
Member

@ota-meshi I'm exploring an alternative syntax for this currently, which would look like this:

<script lang="ts" generics="T, X extends string">
  export let items: T[];
  export let key: X;
</script>

From an implementation-perspective, how hard would it be for svelte-eslint-parser to support this syntax?

(I'm not saying it's decided that we switch to this syntax yet, but I want to have as much feedback as possible before making the decision)

@ota-meshi
Copy link
Member

ota-meshi commented Mar 28, 2023

Thank you for letting me know.
Vue team is also exploring generic attribute, so I'm looking into how to support it with the vue-eslint-parser.
(I'm the maintainer of eslint-plugin-vue and vue-eslint-parser.)

The current idea is to insert virtual code like this:

type T = unknown;
type X = string;

In TypeScript, this might generate the wrong type like the following example code, but I anticipate no problem within the scope of the checks used in typescript-eslint.

<script lang="ts" generics="T extends string | number">
  let val: T = 42 // It's a type error in TypeScript.
</script>

↓ Virtual code

type T = string | number
let val: T = 42 // It's not an error. Also, `val` is inferred to be of type number.

If it works, I think svelte-eslint-parser can handle it the same way.

@wvhulle
Copy link

wvhulle commented May 9, 2023

@ota-meshi I'm exploring an alternative syntax for this currently, which would look like this:

<script lang="ts" generics="T, X extends string">
  export let items: T[];
  export let key: X;
</script>

From an implementation-perspective, how hard would it be for svelte-eslint-parser to support this syntax?

(I'm not saying it's decided that we switch to this syntax yet, but I want to have as much feedback as possible before making the decision)

It would be nice to keep the $$Generic in inside the script. If you have a lot of generics you can't just write them on one line.

And can you keep the types interactive and highlighted as a generics property?

@connerdassen
Copy link

Why is this not supported?
The now obsolete eslint-plugin-svelte3 had this added a year and 8 months ago.
Defining $$Generic globally in eslintrc gets rid of the undefined error but causes problems later on because it is assigned an "any" type, leading to no-unsafe-member-access errors later on when trying to use the type defined by $$Generic

@dummdidumm
Copy link
Member

It's not supported yet because it's still experimental, which is a valid choice.

@probablykasper
Copy link

Would love to update to Svelte 4, but the deprecated eslint-plugin-svelte3 doesn't support it. Was I unknowingly using an experimental feature, or is it just not supported anymore? Are there any plans for when to address it?

@theodorejb
Copy link

@probablykasper Switch to eslint-plugin-svelte.

@probablykasper
Copy link

@theodorejb I would love to, but it sounds like $$Generic is unsupported, without any workaround

@jjnp
Copy link

jjnp commented Sep 3, 2023

Would you be open to a PR to add support for the feature, even if it's still just an RFC? If you are focused on other tasks, which I of course understand, I'd try to look into what I can do to help the feature along. We're using generics in multiple production apps and having this break with the upgrade to svelte-4 is a step back for our project setup...

@ota-meshi
Copy link
Member

I think we can add support for Generics to svelte-eslint-parser as an experimental feature. Please consider submitting a PR. However, please allow it to be opted in using something like parserOptions.
However, $$Generic seems to have been discarded from the RFC, so we won't be adding support for it in the future.

@niemyjski
Copy link

It's pretty bad dev experience when the official docs say to use these features but then linting fails out of the box when using these features...

niemyjski added a commit to exceptionless/Exceptionless that referenced this issue Sep 21, 2023
@jjnp
Copy link

jjnp commented Sep 21, 2023

It's pretty bad dev experience when the official docs say to use these features but then linting fails out of the box when using these features...

Yes... I'm looking into writing a PR to add support for it, but I haven't gotten around to it yet since I'm not yet familiar with developing eslint-plugin related things^^

@AlbertMarashi
Copy link

Yeah also ran into this issue. I thought I was doing something wrong but apparently not

@AlbertMarashi
Copy link

AlbertMarashi commented Sep 25, 2023

Temporary workaround for those people who's T is unknown/generic

<script generics="T" lang="ts">
// TODO: REMOVE WHEN https://github.com/sveltejs/svelte-eslint-parser/issues/306 IS FIXED
// eslint-disable-next-line no-undef, @typescript-eslint/no-explicit-any
type X = T | any

export let options = X[]

This will get the errors to disappear

niemyjski added a commit to exceptionless/Exceptionless that referenced this issue Oct 15, 2023
* Created new sveltekit project

* Added VS Code launch support for svelte.

* Added tailwind and dasiyui

* Added helpful resources

* Added models generated from swagger.

* WIP Added login page

* Updated login

* Fixed launch config and updated login styles

* WIP - login page validation work.

* Added api client.

* Added problem details

* Fixed tests

* Fixed tests

* WIP Problem details.

* Login page refactor

* Some cleanup and get events list page

* Minor

* Store token in localstorage, redirect back to page after login

* Logout page

* Added api launch config

* Fixed formatting.

* Change default proxy port to match API. Some work on OAuth login.

* Login page fixes

* Fixed login page

* Swagger fixes

* Google login working

* Adding other oauth providers

* State is google specific

* Added shared layout for auth pages.

* Few minor oauth updates

* Fixed merge build errors

* Fixed build errors

* Update deps

* Applied linting

* Regenerated models after Nullable reference types

* Added default templates

npx swagger-typescript-api generate-templates -o api-templates

* Updates to client side model generation

* Added ability to pull in fluent validation rules into swagger.

* Updated generation

* more generation improvements

* Improved code gen

* More code gen fixes

* Updated signup model validation

* code gen improvements

* Removed constructor

* Some cleanup of problem details / validation

* Updated swagger config

* WIP: MiniValidation with automatic problem details

DamianEdwards/MiniValidation#51

* Login and logout flow + dashboard with auth detection.

* Reverted some model state changes

* Reworked how api client redirects

* Added websockets and layout auth redirection

* Added samples for both paging and infinity paging

* Updated to tanstack query (other one was deprecated)

* More example resources

* Updated deps

* Added paging helpers to json response

* Updated server response types and base summary model

* Added Events Table component which supports column selection, default summaries (WIP)

* Pushed summary updates

* FetchClient changes to add middleware and make it more generic

* Add global default options to FetchClient, setup global model validator, and global middleware to redirect on 401.

* Some small fixes.

* some fetch updates

* Fixed build warnings

* Early WIP - Taillog component

* Greatly improved summaries

* some query client work

* WIP event tail log

* Always return search tokens

* Updated deps

* Added default public env file

* Conditionally turn off account creation

* Moved oauth to use env

* Added exceptionless client

* Fixed client warnings and errors by adding trace logging.

* changed file type of hook

* Fixed app login

* Fixed summary formatters

* More efficient events tail log.

* Added temporary drawer

* Added live mode, start of pager.

* Moving app base to /next, more FetchClient updates, fix unmount code for eventstail component.

* Some styling

* More style

* Conditionally render summaries

* Added new websocket message component to streamline how we listen to events

* Working on getting both client apps to run together

* Remove lint from build

* Handle fallback for /next client app

* Fixed logo

* Change URL to /next for SPA startup

* Change launchUrl to match SPA url

* Some paging updates

* Added paging summary

* Fixed paging

* Fixed paging

* Fixed the pager on mobile.

* Created generic table component (WIP)

* forward events

https://learn.svelte.dev/tutorial/event-forwarding

* Ensure json doesn't get new line inserted

* Fixed summary tests

* Fixed validation tests

* Fixed all tests

* Fixed bad auto git merge

* Fixed redirect

* Added pagination helper.

* WIP: Added column sorting

* Added column picker

* Added ability to navigate to the first page

* Fixed binding issue

* Fixed callback

* Fixed drawer

* Use store for drawer and for live view

* WIP search and filtering

* Added empty table message.

* Workaround for $$Generic

sveltejs/svelte-eslint-parser#306

* Fixed linting errors, updated deps

* Use global fetch / loading

* Updated deps

* Work arounds for slot forwarding, couldn't reuse let:variable syntax and had to use writables for page.

sveltejs/svelte#7021

* use persisted storage for events column options

* WIP events drawer

* Updated deps

* Rearranged models a bit and pulled in some models from the exceptionless client

* Added components for time formatting

* WIP: Events drawer and overview.

* Fix issue with code formatter changing line endings and causing git issues. Turn daisyui banner off.

* Parallelize build

* Events Sidebar

* Ran formatting

* Try building just exceptionless all in one image during docker build step

* Don't run client tests yet

* Try only building app docker image

* Set artifact retention

* Tweak how we start docker services

* Don't wait for docker compose

* Debug

* Try different approach

* Checkout on deploy

* Try different docker build

* Another parallel approach

* Break out deploy

* More build tweaks

* Add code coverage

* Use run-all to run multiple tasks

* Revert some CSP policy changes

* Comment out unused code to pass lint rules

* Allow http gravatar

* Add missing logo back

* increased sidebar width

* WIP - Error and Simple error stack traces

* Fixed http file

* Updated deps

* Reworked how stack traces are rendered

* removed extra whitespace.

* Whitespace formatting

* More styling fixes

* make stack trace look nicer

* Updated deps

* WIP: Clickable filters with ability to enter custom keyword filters.

* rearranged loading rendering order in component due to global loader.

* Renamed functions

* Added more search + log level component

* Fixed warning

---------

Co-authored-by: Blake Niemyjski <bniemyjski@gmail.com>
@AlbertMarashi
Copy link

AlbertMarashi commented Feb 12, 2024

@Rich-Harris will this be fixed in Svelte 5 because of the new props system?

@dummdidumm
Copy link
Member

$$Generic is deprecated in favor of the generics attribute, which should be supported in this parser at some point. For more info see #306 (comment)

@AlbertMarashi
Copy link

I see. Is there any issues that need resolving that perhaps I could help work on?

@dummdidumm
Copy link
Member

I'll defer to @ota-meshi and @baseballyama for this question

@ota-meshi
Copy link
Member

I think if someone works on this implementation, it can be added as an experimental feature.
#306 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants