search

ni / javascript-styleguide

JavaScript and TypeScript Style Guide
MIT License
9 stars 9 forks source link

Make it easier to adopt strictNullCheck rules #73

Open rajsite opened 2 years ago

rajsite commented 2 years ago

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:

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 commented 2 years ago

Alternate:

Default as strict and make current configs as:

rajsite commented 2 years ago

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 commented 1 year ago

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:

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 commented 1 year ago

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 commented 1 year ago

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.