Vscode: Enable `strictPropertyInitialization`

Created on 30 Jul 2019  ยท  19Comments  ยท  Source: microsoft/vscode

We have enabled strictNullChecks, but the TS compiler doesn't catch the following:

class A {
  prop: number;
  constructor() {
    // forgot to assign to prop, so it will be undefined...
  }
}

Possible fixes:

class A {
  prop: number;
  constructor() {
    this.prop = 5; // Assign the property in the ctor
  }
}
class A {
  prop!: number; // use exclamation mark to suppress the error
  constructor() {
    this._init();
  }
  private _init() {
    this.prop = 5;
  }
}

In order to get errors for such cases, we need to add "strictPropertyInitialization": true to our tsconfig.json.

@microsoft/vscode it would be nice if we could tackle some of these errors during debt week.

[edit 11.11]: There are 8 errors left:

  • [ ] vs/base/browser/ui/tree/asyncDataTree.ts @joaomoreno
  • [ ] src/vs/workbench/api/common/extHostTypes.ts @jrieken
  • [ ] src/vs/workbench/api/common/extHostWebview.ts @mjbvz
debt engineering

Most helpful comment

Adding it to next milestone so it shows up in my issue query for debt week.
If we do not tackle it completely we can always move it to the next one afterwards.

All 19 comments

Adding it to next milestone so it shows up in my issue query for debt week.
If we do not tackle it completely we can always move it to the next one afterwards.

Went through a bunch of these.

I am seeing quite a bit of these in the code:

StrictNullOverride Nulling out ok in dispose

I think that is not a good pattern and we should never do that, because disposing something is no guarantee that the code is not used anymore. This is causing NPEs left and right still.

image

Yeah, I never understood why "nulling" during dispose is helpful? Is this a defence to prevent leakage when the entity disposing an object still holds on to the object?

Since many of these seem to originate from @joaomoreno , let's wait when he returns.

These are all from the old days, when we were all doing that. It doesn't make any sense. When strict null came in, these lines should've just been deleted.

Assigning a few more pepole:

  • [x] @RMacfarlane comments
  • [ ] @chrmarti quick input, walkthrough
  • [x] @aeschli theme service
  • [x] @joaomoreno scm, tree
  • [ ] @sbatten menubar / title
  • [x] @mjbvz webview
  • [ ] @alexandrudima parameter hints, editor options

I know maybe you are not the original author, but we need more help to get this down to 0. Sandy is out in October and I will not fix the remaining 437 ones alone. Thanks ๐Ÿ‘

Added @weinand for debug

  • [x] extHostTypes Task @alexr00
  • [x] extHostDebug @isidorn @weinand
  • [ ] extHostQuickOpen @chrmarti
  • [x] extHostWebview @mjbvz

:tophat:

Getting there:

image

@chrmarti if you do yours I can maybe look into Sandeeps.

@bpasero Done. ๐Ÿ‘

Looks like Sandy is back :)

Yes, I am back and on to this ๐Ÿ˜„

โœ…

Remaining:
image

@mjbvz d.ts & webview
@joaomoreno async tree
@jrieken extHostTypes (call hierarchy related it seems)

Fixed extHostWebview again. Down to four errors in three files:

  • viewsViewlet.ts
  • `asyncDataTree.ts
  • extHostTypes.ts

Ping @alexr00 for viewsViewlet.ts

I went ahead and resolved those so that we do not have to play the catch up game all the time ๐Ÿ‘

Was this page helpful?
0 / 5 - 0 ratings