irossimoline/angular4-material-table

When can TableElement.currentData be undefined?

Closed this issue · 4 comments

hossameldeen commented

I have strictNullChecks enabled, and I have the following code:

// html
<button (click)="delete(row)">

// ts
delete(row: TableElement<X>) {
  // do some business with one of the properties of currentData: for example logging it:
  console.log(row.currentData.someProperty)
  row.delete()
}

When I compile it, I get this error:

error TS2532: Object is possibly 'undefined'

That's because currentData is optional in TableElement class, so it could possibly be undefined and I can't access someProperty on undefined.

My question: at what cases would currentData be undefined? I'm bewildered because originalData isn't optional. Can currentData just be a required property like originalData?

Thanks!

irossimoline commented

Hi @hossameldeen,

I think you have found an issue. The thing is that originalData could be undefined, but currentData shouldn't: the first one is only populated when editing an element, and the last one is the one that holds the row data, so it should not be undefined. I'm not with my PC near now, but I can fix it and push soon.

  • 👍1
irossimoline commented

Hi @hossameldeen,

I've published a new version with this fix. :)
If you have any question please let me know.

  • 👍1
hossameldeen commented

@irossimoline , I was writing this comment when you pushed the fix :-) Thanks for the prompt response and for the awesome package!

I've checked it, will upgrade. Nope, no more questions 👍 , you can close.

By the way, I'm a fan of strictNullChecks. So, I've tried to enable it on your project and resolve the errors. One error that particularly caught my eye is this:

  cancelOrDelete(): void {
    if (this.id == -1 || !this.editing)
      this.delete();
    else {
      this.currentData = this.originalData; // I've made currentData required while originalData optional, so an error came up here for assigning optional to required
      this.editing = false;
      this.validator.disable();
    }
  }

Of course, the if-condition above !this.editing makes sure that we're in editing mode in the other branch & thus have originalData. But how to make sure that editing == true -implies-> originalData !== undefined across the whole code?

For the purposes of technical curiosity, I've made a solution & reached it to passing compilation (didn't even test). I probably don't recommend merging it into the package since it'll break backward compatibility and may need a bit refactoring. But anyway, I'm opening a PR with it in case you want to check it out :-)

Thanks again :-)

  • 👍1
irossimoline commented

Cool @hossameldeen!

I'm gonna close this issue, and lets talk about the code improvement proposed directly on the PR.
Thank you in advance for contributing with this project! 😃

  • 👍1