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

Make it easier to adopt strictNullCheck rules #73

Open
rajsite opened this issue Dec 1, 2021 · 5 comments
Open

Make it easier to adopt strictNullCheck rules #73

rajsite opened this issue Dec 1, 2021 · 5 comments
Assignees

Comments

@rajsite
Copy link
Member

rajsite commented Dec 1, 2021

Right now the topic links to a search for the [strict-null-checks] flag but when you visit those spots in source it is not immediately clear what the recommended config with strict null checks enabled should be (just guessed that 'off' should toggle to 'error', etc, but that might not be clear for first time eslint users).

We can either put recommended configs in the comments or go one step further and create strict versions of the rules, ie:

  • @ni/eslint-config-typescript/strict
  • @ni/eslint-config-typescript/requiring-type-checking-strict
  • @ni/eslint-config-angular/strict
  • @ni/eslint-config-angular/requiring-type-checking-strict

I think having dedicated rulesets for strict mode will be handy to encourage consistent strict-mode usage in apps and make it easier to adjust the strict mode configuration going forward. Will also make it easier for new projects trying to start with good strict mode settings.

Edit: Creating strict configs and having a Web Chapter presentation on strict-null-checks might be a good way to share the knowledge

@rajsite
Copy link
Member Author

rajsite commented Jan 6, 2022

Alternate:

Default as strict and make current configs as:

  • @ni/eslint-config-typescript/less-strict
  • @ni/eslint-config-typescript/requiring-type-checking-less-strict
  • @ni/eslint-config-angular/less-strict
  • @ni/eslint-config-angular/requiring-type-checking-less-strict

@rajsite
Copy link
Member Author

rajsite commented Jan 29, 2022

With strictNullChecks the eslint default-case and typescript-eslint/switch-exhaustiveness-check are incompatible. Should the strict ruleset disable default-case in favor of switch-exhaustiveness-check? consistent-return might also have an issue

@rajsite rajsite self-assigned this Apr 28, 2022
@rajsite
Copy link
Member Author

rajsite commented Feb 23, 2023

Re-upping this issue for 2023 now that we've have a bit of runtime and use strict mode in more projects.

New proposal:

  1. Make our default configuration files strict (encourage strict rules as the default)
  2. Merge the requiring-type-checking rules into the default rules (doing code searches it does not seem like repos opt out of the requiring-type-checking rule sets)
  3. Add the following configuration files which extend the default files:
  • @ni/eslint-config-typescript/legacy
  • @ni/eslint-config-angular/legacy

So the end result would be the following configs:

  • @ni/eslint-config-typescript
  • @ni/eslint-config-typescript/legacy
  • @ni/eslint-config-angular
  • @ni/eslint-config-angular/legacy

The intentions are to make the out-of-the-box configurations extending the base rules strict and aligned by default.
Thoughts / votes @TrevorKarjanis @jattasNI @mure?

@jattasNI
Copy link
Collaborator

To be explicit about it: this would be a breaking change (major version bump) and clients would own the work of migrating their config files to the new config names?

If so we should communicate before and after this change is made via the UI WG channel and meeting.

In case it's not obvious, my 👍 above indicates a vote for the proposal.

@TrevorKarjanis
Copy link
Collaborator

TrevorKarjanis commented Mar 8, 2023

I can't remember exactly, but I have a hunch that my concern with the original proposal was that people wouldn't associate "strict" with strict mode and instead think we have a less strict option which is not true. This may not be the case as much now that strict mode is more prominent. The new proposal eliminates this issue anyway, and I definitely think the primary configurations should support strict mode. My 👍 above indicates a vote for the proposal.

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