Damir's Corner.

Detect Buggy Property Assignment Early

I was recently invited to look at a piece of seemingly working JavaScript code. The problem was that the TypeScript compiler was complaining about it after it was annotated with type information. In the end, these errors uncovered a hidden bug in the code.

The code was a React onChange handler:

handleInputChange = (event) => {
  const target = event.target;
  const name = target.name;
  const value = name === 'visible' ? target.checked : target.value;
  var data = this.state.data;
  data[name] = value;
  this.setState({
    data,
  });
};

Adding type information wasn't a big deal:

interface Entry {
  id: string;
  description: string;
  minPrice: number;
  maxPrice: number;
  visible: boolean;
}

export class FakeComponent {
  state: { data: Entry };

  // ...

  handleInputChange = (event: React.ChangeEvent<HTMLInputElement>) => {
    // ...
  };
}

However, the compiler complained about the following line of code

data[name] = value;

The error?

Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'Entry'.

No index signature with a parameter of type 'string' was found on type 'Entry'.

Sure, the problem can be avoided by disabling type checking for this particular assignment:

var data = this.state.data as any;

Although the resulting JavaScript would be identical to the original code, this defeats the purpose of using TypeScript in the first place. We want to take advantage of type checking, not avoid it.

The reported error is caused by the fact that name is of type string but only some of its values are valid field names of Entry. This can be resolved with a type guard:

if (
  name === 'id' ||
  name === 'description' ||
  name === 'minPrice' ||
  name === 'maxPrice' ||
  name === 'visible'
) {
  data[name] = value;
}

This would make sure that no property assignment happens if the value of name doesn't match an Entry field name. TypeScript protects us from adding extra properties to the object. Yes, there might be no harm in doing that and we might also know that the names of input elements match the field names, but the TypeScript compiler doesn't know that and the above type guard is the only way to be sure.

Even with this change, the TypeScript compiler still complains:

Type 'string | boolean' is not assignable to type 'never'.

Type 'string' is not assignable to type 'never'.

The problem is that Entry fields are of different types and the code still doesn't ensure that the assigned value matches the type of the underlying field. To fix the error, we must make sure that the types of values match:

if (name === 'visible') {
  data[name] = target.checked;
} else if (name === 'id' || name === 'description') {
  data[name] = target.value;
} else if (name === 'minPrice' || name === 'maxPrice') {
  data[name] = parseInt(target.value, 10);
}

With this change, there are no more TypeScript errors. The changes also propagated to the generated JavaScript:

this.handleInputChange = (event) => {
  const target = event.target;
  const name = target.name;
  var data = this.state.data;
  if (name === 'visible') {
    data[name] = target.checked;
  } else if (name === 'id' || name === 'description') {
    data[name] = target.value;
  } else if (name === 'minPrice' || name === 'maxPrice') {
    data[name] = parseInt(target.value, 10);
  }
  this.setState({
    data,
  });
};

In the process of fixing the TypeScript errors, we also fixed a bug that caused a string to be assigned to minPrice and maxPrice instead of a parsed number. Thanks to JavaScript type coercion this could manifest in a nasty hidden bug:

const component = new FakeComponent({
  id: '1',
  description: 'initial description',
  minPrice: 10,
  maxPrice: 20,
  visible: true,
  photos: [],
});

const event = {
  target: {
    name: 'minPrice',
    value: '12',
  },
} as React.ChangeEvent<HTMLInputElement>;

component.handleInputChange(event);
const entry = component.state.data;
const midPrice = entry.minPrice + entry.maxPrice;

expect(midPrice).toBe(16); // Received: 610

You can check the full code for the above example in my GitHub repository.

Although TypeScript sometimes seems to complain about working JavaScript code, it's always because of a type mismatch of some kind. It might be as innocent as a precondition not expressed in TypeScript code (what input fields are available in our example). But it might also be a real bug that you'd struggle to find otherwise (having a string value where a number is expected in our example). It's usually worth it to get to the bottom of each error instead of simply opting out of type checking.