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

[RFC] Selectively Ignoring Diagnostics #1354

Closed
Tracked by #3887
aldonline opened this issue Oct 14, 2020 · 1 comment
Closed
Tracked by #3887

[RFC] Selectively Ignoring Diagnostics #1354

aldonline opened this issue Oct 14, 2020 · 1 comment

Comments

@aldonline
Copy link
Contributor

aldonline commented Oct 14, 2020

Currently there is no way to turn off diagnostics.
In other words, the VSCode extension and rw check will show a fixed list of errors/warnings, and the user has no option but to "correct" them in order to make them go away.

I can think of a few scenarios where being able to selectively ignore errors can be useful:

  1. When a RW developer is knowingly going outside of the "norm" and wants to silence an error/warning
  2. When a new diagnostic check is introduced (in a new Redwood release) that further restricts what was, until previous Redwood versions, not considered an error or warning
  3. When the logic behind a diagnostic check has a bug and is mistakenly classifying something as an error. This is likely to happen at this early stage as diagnostics are usually trying to check, at compile time, some runtime behavior which is likely still under development or under-specified.

Proposal

General Settings

Project wide ignore settings can be added to redwood.toml.

[web]
  port = 8910
[diagnostics]
  notfound_route_cannot_be_private = false
  env_vars_must_be_explicitly_declared_in_redwood_toml = false

Alternatively, we could use check instead of diagnostics (since we seem to favor rw check, right?)

[web]
  port = 8910
[check]
  notfound_route_cannot_be_private = false
  env_vars_must_be_explicitly_declared_in_redwood_toml = false

Granular Settings

Just like es/ts-lint, we should also be able to disable checks on a per-file and per-line basis:

Per file:

/* rwcheck-disable some_diagnostic_code  */

Per line:

// rwcheck-disable-next-line some_diagnostic_code

Note: This is harder to implement, so we would initially only support settings on redwood.toml

Note on Error Codes

Traditionally, error codes are opaque numbers. For example:

  • HTTP 501: Not Implemented
  • TypeScript 1002: Unterminated string literal

This is mostly due to historical reasons and/or restrictions that don't apply in our case (we don't have to optimize for size or transport).

I propose we use more descriptive string codes. This is what modern linters use, and they will be more familiar to users trying to write "ignore" rules.

The "master" list of diagnostic codes and their descriptions will be located somewhere in the repo, and it can be used to generate reference documentation.

For example:

// somewhere in the redwood codebase...
const diagnostics = {
  invalid_route_path_syntax: {
    message: "Invalid Route Path Syntax",
    doc: "https://redwoodjs.com/docs/redwood-router#route-parameters",
    category: "router",
  },
  notfound_route_cannot_be_private: {
    message: "The 'notfound' route cannot be private",
    category: "router",
  },
  env_vars_must_be_explicitly_declared_in_redwood_toml: {
     category: "config"
  },
  env_vars_explicit_decl: {
    // code aliases allows us to rename codes over time without breaking previous projects
    aliasFor: "env_vars_must_be_explicitly_declared_in_redwood_toml"
  }
}

Thoughts? Ideas? :)
Please share!

@jtoar
Copy link
Contributor

jtoar commented May 5, 2022

See #3887 (comment).

@jtoar jtoar closed this as completed May 5, 2022
Repository owner moved this from Backlog to Done in Main May 5, 2022
@jtoar jtoar removed this from Main May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants