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

Svelte 4 Generic Syntax in docs #245

Closed
Skylli202 opened this issue Aug 7, 2023 · 20 comments
Closed

Svelte 4 Generic Syntax in docs #245

Skylli202 opened this issue Aug 7, 2023 · 20 comments
Labels
documentation Improvements or additions to documentation

Comments

@Skylli202
Copy link

Skylli202 commented Aug 7, 2023

Hi,

I experimented the guide about componentization, once I made my way through (very helpful guide btw, thanks a lot). I had an issue with ESLint. ESlint was not happy about this : type T = $$Generic<AnyZodObject>;, in the TextField.svelte component.

After doing some research, I think (I'm not a Svelte pro yet!) that this is the old way of defining generics. The new version would be within the script tag <script lang="ts" generics="T extends AnyZodObject">. But my ESLint is not happier by this way of defining the generic.

I would be happy to make a PR to edit the documentation, but I'd like to have my take validated first.

Thanks in advance.


GitHub issues related to this syntax issues :
sveltejs/eslint-plugin-svelte3#127
sveltejs/svelte-eslint-parser#306
sveltejs/eslint-plugin-svelte#521 <-- much recent and interesting

@ciscoheat
Copy link
Owner

Hi, are you using some additional rules or special config? It works with no warnings for me.

@ciscoheat
Copy link
Owner

The old syntax, that is. Haven't tried the new one.

@Skylli202
Copy link
Author

Skylli202 commented Aug 9, 2023

Hi,

I'm running on a default new sveltekit project. Here is my configuration, .eslintrc.cjs :

module.exports = {
	root: true,
	extends: [
		'eslint:recommended',
		'plugin:@typescript-eslint/recommended',
		'plugin:svelte/recommended',
		'prettier'
	],
	parser: '@typescript-eslint/parser',
	plugins: ['@typescript-eslint'],
	parserOptions: {
		sourceType: 'module',
		ecmaVersion: 2020,
		extraFileExtensions: ['.svelte']
	},
	env: {
		browser: true,
		es2017: true,
		node: true
	},
	overrides: [
		{
			files: ['*.svelte'],
			parser: 'svelte-eslint-parser',
			parserOptions: {
				parser: '@typescript-eslint/parser'
			}
		}
	],
	globals: {
		$$Generic: 'readonly'
	}
};

Please note that the globals keys is a workaround I added after creating this issue (found this solutions in one of the others issues linked in my original post).

@walker-tx
Copy link

Per the migration (svelte 3 -> svelte 4) docs, I copy+pasta'd the eslintrc and tsconfig files from npm create svelte@latest, and I'm having this issue as well. Would love to get rolling with the new generic definitions, too.

@ciscoheat
Copy link
Owner

ciscoheat commented Aug 22, 2023

These two components gives me no linting errors with eslint-plugin-svelte 2.32.4 and svelte 4.2.0:

<script lang="ts" generics="T extends AnyZodObject">
  import type { z, AnyZodObject } from 'zod';
  import type { ZodValidation, FormPathLeaves } from '$lib';
  import { formFieldProxy, type SuperForm } from '$lib/client';

  export let form: SuperForm<ZodValidation<T>, unknown>;
  export let field: FormPathLeaves<z.infer<T>>;

  const { path, value, errors, constraints } = formFieldProxy(form, field);
</script>

<label>
  {String(path)}<br />
  <input
    type="text"
    data-invalid={$errors}
    bind:value={$value}
    {...$constraints}
    {...$$restProps}
  />
</label>
{#if $errors}<span class="invalid">{$errors}</span>{/if}

<style>
  .invalid {
    color: orangered;
  }
</style>
<script lang="ts" generics="T extends AnyZodObject">
  import type { z, AnyZodObject } from 'zod';
  import type { FormField } from '$lib';

  export let type = 'text';
  export let label: string;
  export let field: FormField<T, keyof z.infer<T>>;
  export let placeholder = '';

  function setType(el: HTMLInputElement) {
    el.type = type;
  }

  $: value = field.value;
  $: errors = field.errors;
  $: constraints = field.constraints;
</script>

<label>
  {label}<br /><input
    use:setType
    {placeholder}
    bind:value={$value}
    name={field.name}
    {...$constraints}
    data-invalid={$errors}
  />
  {#if $errors}<span class="invalid">{$errors}</span>{/if}
</label>

<style>
  [data-invalid],
  .invalid {
    color: red;
  }
</style>

If anyone of you can confirm that this approach works with your own components, I'll update the documentation.

@Skylli202
Copy link
Author

I'll try this asap and come back to you!

@Skylli202
Copy link
Author

Also, great idea to use that setType function. I tried myself on my project to make this component generic without success. That's definitely a good update of the doc regardless of the eslint issue.

@ciscoheat
Copy link
Owner

Oops, that snuck in by mistake. Setting the type dynamically will cause trouble with bindings to type number, so I cannot recommend it. An if statement is better. Sorry. :)

@Skylli202
Copy link
Author

Skylli202 commented Aug 22, 2023

So, I currently have eslint-plugin-svelte: 2.32.4 installed alongside with svelte: 4.2.0 in my package-lock.json.

To do your test, used the following .eslintrc.cjs I believe it is the default one but please confirm / double check this (I do not know how to regenerate the default one).

Edit: my .eslintrc.cjs is the same as the one described in the eslint manual migration svelte 3 to 4

.eslintrc.cjs
module.exports = {
	root: true,
	extends: [
		'eslint:recommended',
		'plugin:@typescript-eslint/recommended',
		'plugin:svelte/recommended',
		'prettier'
	],
	parser: '@typescript-eslint/parser',
	plugins: ['@typescript-eslint'],
	parserOptions: {
		sourceType: 'module',
		ecmaVersion: 2020,
		extraFileExtensions: ['.svelte']
	},
	env: {
		browser: true,
		es2017: true,
		node: true
	},
	overrides: [
		{
			files: ['*.svelte'],
			parser: 'svelte-eslint-parser',
			parserOptions: {
				parser: '@typescript-eslint/parser'
			}
		}
	]
};

Your first example gave me the following errors:

/path/to/project/src/lib/components/Test1.svelte
  2:19  warning  'AnyZodObject' is defined but never used  @typescript-eslint/no-unused-vars
  6:43  error    'T' is not defined                        no-undef
  7:43  error    'T' is not defined                        no-undef

Your second example gave me the following errors:

/path/to/project/src/lib/components/Test2.svelte
  2:19  warning  'AnyZodObject' is defined but never used  @typescript-eslint/no-unused-vars
  7:30  error    'T' is not defined                        no-undef
  7:47  error    'T' is not defined                        no-undef

@ciscoheat
Copy link
Owner

I created a new Sveltekit project from scratch, but couldn't get those errors appearing until I ran pnpm eslint . manually. Anyone getting them directly in VS Code?

In any case, there must exist a type definition of AnyZodObject in a module context script, for it to work. So this finally gives no eslint warnings or errors:

<script lang="ts" context="module">
  import type { AnyZodObject } from 'zod';
  type T = AnyZodObject;
</script>

<script lang="ts" generics="T extends AnyZodObject">
  import type { z } from 'zod';
  import type { ZodValidation, FormPathLeaves } from 'sveltekit-superforms';
  import { formFieldProxy, type SuperForm } from 'sveltekit-superforms/client';

  export let form: SuperForm<ZodValidation<T>, unknown>;
  export let field: FormPathLeaves<z.infer<T>>;

  const { path, value, errors, constraints } = formFieldProxy(form, field);
</script>

<label>
  {String(path)}<br />
  <input
    type="text"
    data-invalid={$errors}
    bind:value={$value}
    {...$constraints}
    {...$$restProps}
  />
</label>
{#if $errors}<span class="invalid">{$errors}</span>{/if}

<style>
  .invalid {
    color: orangered;
  }
</style>

@Skylli202
Copy link
Author

I created a new Sveltekit project from scratch, but couldn't get those errors appearing until I ran pnpm eslint . manually. Anyone getting them directly in VS Code?

Sorry, I think we misunderstood each other. My VSCode does not fire any errors. It's only the npm run lint command (default in any new sveltekit project made with npm create svelte@latest my-app) who does.

It is inconvenient as it makes my jobs fail (GitLab version of GitHub Actions), but as stated above, I have a workaround with the ESLint globals key in configuration.

While I can't try your workaround at the moment, I'll definitely give it a shot and come back to you about it.

Thanks for your valuable work on that matter.

@ciscoheat
Copy link
Owner

It's not a workaround, the latest is not giving any eslint errors, either from the command line or in VS Code. :)

@Skylli202
Copy link
Author

When I said workaround, I was referring to the globals key in ESLint configuration. If this is a fix, and you have not already done so. I would be happy to participate in a PR on the documentation website repo.

@BenocxX
Copy link

BenocxX commented Aug 31, 2023

Anyone getting them directly in VS Code?

I was getting them inside VS Code.


I confirm that with the provided fix everything works on my end. Thanks a lot!

@ciscoheat
Copy link
Owner

@BenocxX great that it works. Any extra configuration, preprocessing for you, to have it displayed in VS Code?

@ciscoheat
Copy link
Owner

ciscoheat commented Aug 31, 2023

Got the answer now, the eslint plugin needs to be configured in VS Code:

{
  "eslint.validate": ["javascript", "typescript", "svelte"]
}

Edit: For me it was enough with just "svelte" in the array, typescript seems to work anyway.

@BenocxX
Copy link

BenocxX commented Aug 31, 2023

Any extra configuration, preprocessing for you, to have it displayed in VS Code?

I have the ESLint VS Code extension and I have a the following configuration:

module.exports = {
  root: true,
  extends: [
    'eslint:recommended',
    'plugin:@typescript-eslint/recommended',
    'plugin:svelte/recommended',
    'prettier',
  ],
  parser: '@typescript-eslint/parser',
  plugins: ['@typescript-eslint'],
  parserOptions: {
    sourceType: 'module',
    ecmaVersion: 2020,
    extraFileExtensions: ['.svelte'],
  },
  env: {
    browser: true,
    es2017: true,
    node: true,
  },
  overrides: [
    {
      files: ['*.svelte'],
      parser: 'svelte-eslint-parser',
      parserOptions: {
        parser: '@typescript-eslint/parser',
      },
    },
  ],
  rules: {
    'arrow-spacing': ['warn', { before: true, after: true }],
    'brace-style': ['error', '1tbs', { allowSingleLine: true }],
    'comma-dangle': ['error', 'only-multiline'],
    'comma-spacing': 'error',
    'comma-style': 'error',
    curly: ['error', 'multi-line', 'consistent'],
    'dot-location': ['error', 'property'],
    'handle-callback-err': 'off',
    indent: ['error', 2],
    'keyword-spacing': 'error',
    'max-nested-callbacks': ['error', { max: 4 }],
    'max-statements-per-line': ['error', { max: 1 }],
    'no-console': 'off',
    'no-empty-function': 'error',
    'no-floating-decimal': 'error',
    'no-inline-comments': 'off',
    'no-lonely-if': 'error',
    'no-multi-spaces': 'error',
    'no-multiple-empty-lines': ['error', { max: 2, maxEOF: 1, maxBOF: 0 }],
    'no-trailing-spaces': ['error'],
    'no-unused-vars': 'off',
    '@typescript-eslint/no-unused-vars': 'off',
    'no-var': 'error',
    'object-curly-spacing': ['error', 'always'],
    'prefer-const': 'error',
    semi: ['error', 'always'],
    'space-before-blocks': 'error',
    'space-before-function-paren': [
      'error',
      {
        anonymous: 'always',
        named: 'never',
        asyncArrow: 'always',
      },
    ],
    'space-in-parens': 'error',
    'space-infix-ops': 'error',
    'space-unary-ops': 'error',
    'spaced-comment': 'error',
    yoda: 'error',
  },
};

@ciscoheat ciscoheat added the documentation Improvements or additions to documentation label Sep 1, 2023
@Skylli202
Copy link
Author

Hi @ciscoheat, I can confirm that the module way fixed my issue and I have been able to remove my workaround.

Could you have a use of help on a PR for the documentation website ?

@ciscoheat
Copy link
Owner

That would be nice, thank you.

@ciscoheat
Copy link
Owner

Docs updated on the components page now.

github-merge-queue bot pushed a commit to dfinity/nns-dapp that referenced this issue May 27, 2024
# Motivation

We want to use the `ResponsiveTable` with different types of data.

# Changes

Declare the `tableData` in `ResponsiveTable` and `rowData` in
`ResponsiveTableRow` to be of a generic type.
I found this syntax
[here](ciscoheat/sveltekit-superforms#245 (comment))
and it seems to satisfy both the linter and the compiler.

# Tests

Existing tests pass.
I've also tested this with a preliminary neurons table in another
branch.

# Todos

- [ ] Add entry to changelog (if necessary).
not necessary
github-merge-queue bot pushed a commit to dfinity/nns-dapp that referenced this issue May 27, 2024
# Motivation

We want to use the `ResponsiveTable` with different types of data.

# Changes

Declare the `tableData` in `ResponsiveTable` and `rowData` in
`ResponsiveTableRow` to be of a generic type.
I found this syntax
[here](ciscoheat/sveltekit-superforms#245 (comment))
and it seems to satisfy both the linter and the compiler.

# Tests

Existing tests pass.
I've also tested this with a preliminary neurons table in another
branch.

# Todos

- [ ] Add entry to changelog (if necessary).
not necessary
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

4 participants