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

Use onwarn and other options from svelte.config.js for every tool #855

Open
non25 opened this issue Mar 3, 2021 · 12 comments
Open

Use onwarn and other options from svelte.config.js for every tool #855

non25 opened this issue Mar 3, 2021 · 12 comments

Comments

@non25
Copy link

non25 commented Mar 3, 2021

Currently there are three places where I should configure ignores for svelte's a11y warnings:

  1. language server

    lspconfig.svelte.setup{
      cmd = { "yarn", "svelteserver", "--stdio" };
      on_attach = on_attach;
      settings = {
        svelte = {
          plugin = {
            svelte = {
              compilerWarnings = {
                ["a11y-no-onchange"] = "ignore"
              }
            }
          }
        }
      }
    }
  2. svelte-check, which makes package.json look ugly when configured and this is only place to configure it

    {
      "val": "svelte-check --compiler-warnings a11y-no-onchange:ignore",
      "validate": "yarn val --threshold warning && yarn tsc --noEmit"
    }
  3. eslint

    settings: {
      'svelte3/ignore-warnings': ({ code }) => code === 'a11y-no-onchange',
      // ...
    },

I would love to configure it in one place: svelte.config.js. It can't be avoided because of svelte-preprocess and also these tools are reading it anyway for svelte-preprocess, why not use it for onwarn?

Also in the current situation I can't make per-project configurations for language server.

Ideally, I would like to use svelte.config.js both in language tools and bundler and configure everything svelte-related there.
This stuff too: https://github.com/sveltejs/language-tools/tree/master/packages/svelte-vscode#settings

@benmccann
Copy link
Member

Males sense to me. I imagine the hard part will be agreeing on naming / how exactly it should be specified

@dummdidumm
Copy link
Member

It makes sense for things like warnings. I disagree that all language server settings should be read from it, that should be the responsibility of the IDE to provide those and integrate with the language server.

@non25
Copy link
Author

non25 commented Mar 4, 2021

So using onwarn in it's current form is acceptable direction to go ?
To be compatible with existing configs, we could implement similar onwarn callback interface for intercepting warnings like it is done currently in svelte plugins for bundlers.

The reason for having maybe not all, but some configuration for language server in svelte.config.js is that at least format options are usually configured on per-project basis. 🤔

https://github.com/sveltejs/language-tools/tree/master/packages/svelte-vscode#sveltepluginsvelteformatenable

@dummdidumm
Copy link
Member

dummdidumm commented Mar 4, 2021

Formatting options should be put into a .prettierrc file as the extension makes use of prettier in combination with a prettier plugin to accomplish formatting. https://www.npmjs.com/package/prettier-plugin-svelte

@non25
Copy link
Author

non25 commented Mar 4, 2021

I have prettier as a pre-commit hook. Configuration is in package.json.

{
  "prettier": {
    "singleQuote": true,
    "printWidth": 80,
    "plugins": [
      "prettier-plugin-svelte"
    ]
  }
}

Is there something special in prettier invocation from the LS? Or it should load this configuration like it usually does? 🤔

@dummdidumm
Copy link
Member

This will work, too. My point was that the formatting options should not be the concern of svelte.config.js.

@non25
Copy link
Author

non25 commented Mar 4, 2021

I agree, now I don't see anything else other than onwarn to reasonably move to the svelte.config.js.

There's not much to decide left.

Do making onwarn callback in every tool sounds like a good solution?
Unified configuration compatible with current one in the bundlers.

@Conduitry
Copy link
Member

I'm rather concerned about adding svelte.config.js support to things that already have well established mechanisms for configuration. In ESLint, for example, plugins are just passed the configuration object that ESLint has already loaded. This could be the result of the merging of several hierarchical .eslintrc.js files (or JSON files, or YAML files, or the contents of the eslintConfig keys in package.json files), and I don't think ESLint tells the plugin where any of these files were, so I don't know where the plugin ought to look for svelte.config.js. There's a single unified way for configuration to be handled, and ESLint core wants to be responsible for it.

@non25
Copy link
Author

non25 commented Mar 4, 2021

So the solution is:

  1. Teach my LS client to read config files from the workspace and pass configuration to the LS like svelte-vscode does.
  2. Import configuration from svelte.config.js and use it in .eslintrc.js somehow. Make a list with ignores and use it in both functions or something.
  3. Teach svelte-check to load configuration from workspace.

Looks like the only thing worth modifying is svelte-check then ?

Other than svelte-check, it feels like it is doable in the userland. 🤔

@dummdidumm
Copy link
Member

dummdidumm commented Mar 4, 2021

Candidates for harmonization of onwarn in my opinion are: language-server, svelte-check, rollup plugin, webpack plugin.

@vwkd
Copy link

vwkd commented Aug 10, 2021

Not sure if I'm having a related issue. I want to turn off CSS unused selector stripping since I'm including markup dynamically using {@html}. Currently there seems to be no way to turn it off? I'd appreciate a setting in svelte.config.js to toggle it off.

@jasonlyu123
Copy link
Member

It doesn't make sense to hide this warning in this context. As far as I know, The scoped CSS won't apply to elements injected by {@html}. You would have to use the :global modifier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants