Vscode: Decide on undefined vs null

[bpasero]

Once we are done with the strict null work I would like to consider using just undefined and not undefined + null in our code.

TypeScript for example explicitly states in their coding guidelines, do not use null: https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines#null-and-undefined

I personally feel like null should not be used but just undefined and we should adopt our code towards undefined.

@Microsoft/code thoughts?

[bpasero]

@Microsoft/vscode for input

[jrieken]

Yes, 👍 for undefined because that's what you get when doing array[offBoundIndex] or map.get(badKey). There are a few exceptions like regexp.exec but I do think that undefined is the default JavaScript-way

[joaomoreno]

I'm not so sure this is feasible.

My feeling is that undefined is the most used, since it is also the default variable value: let foo;, or what you get when you delete map[key]. So, we're definitely going to have to deal with it eventually.

But I'm pretty sure a lot of other library calls can null, not just the RegExp example. document.querySelection('does not exist') returns null, for example.

Here's how it works in my mind:

• I use undefined to represent the absence of a value;
• I use null to represent nothingness.

For example:

interface Node<T> {
    value: T | null;
    next?: Node<T>;
}

A node can hold a value T or it can hold nothing. It shouldn't hold an undefined value, since we want nodes to always hold something... even if that something is nothing. Yet, as part of a linked list, a node may or may not have a next node defined.

🤷‍♂️

[jrieken]

That node sample falls apart for Node<null | undefined> but yeah I get your point. Generally, I try to be JavaScript'ish (and DOM is a lib that browsers provide, unlike Math or Map which JS itself defines) but I also understand that it will be hard to reach total happiness on the topic. What we should avoid is IMO is the following

function compare(a: SomeInterface | null, b: SomeInterface | null) { ... }

compare(someValue, map.has(someKey) ? map.get(someKey) : null)

In that sample the compare function is too strict in what it accepts, making it cumbersome to use. In general the rule "be relaxed in what you accept, be strict in what you return" should apply.

[bpasero]

I am not sure we can fully eliminate null in our system, I get the idea for some use cases. But I think undefined should be the general thing and null the exception if there is good reasons for it. At least in my code I feel that I sometimes return null even though it could as well be undefined and that now triggers something | null annotations through the strict null work with no real reasoning behind it.

One thing I remember from ES5 and TS default values (function foo(something = true)) for parameters is that if you pass in undefined, the default parameter would be used, but if you pass in null it would not. Not sure how that changed with ES6, but this was often a source of errors for me.

[Tyriar]

I think "prefer undefined over null" should be as strict as we should get here.

[mjbvz]

If you need to convert null to undefined or convert undefined to null in order to make strict null checking happy, please use the withNullAsUndefined and withUndefinedAsNull helper functions I added. These functions makes sure the conversion is consistent and also tags places that we should investigate de-nulling

[dbaeumer]

Regarding

interface Node<T> {
    value: T | null;
    next?: Node<T>;
}

I still do this with undefined in the following way:

interface Node<T> {
    value: T | undefined;
    next?: Node<T>;
}

since it is different from

interface Node<T> {
    value?: T;
    next?: Node<T>;
}

in terms of object creation. The first one forces

{
   value: undefined
}

whereas the second allows {} to be a valid value for Node which cause a lot of problems.

[bpasero]

Relevant issue in the TS repository where they also explain why they were able to avoid | null in TS. Basically because they do not rely on DOM APIs: https://github.com/Microsoft/TypeScript/issues/9653#issuecomment-232181143

[alexdima]

Before making a decision to go with undefined, I would like to ask that someone reach out to the v8 mailing list or create a definitive benchmark that shows that using undefined in object fields has the same performance impact as using null, specifically around hidden classes and deoptimizations.

I have spent a lot of time in the past in IRHydra (which is now unfortunately defunkt) and have painstakingly went through editor hot paths and deoptimization errors and made sure to not mutate the v8 hidden classes that are "hot". That often meant ensuring each and every field is initialized in the ctor, with null where pointers would later be used.

I would be terribly disappointed if all that work goes to waste because of a subjective preference towards undefined.

In other words, before we even consider expressing personal preference on the topic, we should make sure the runtime works the same way for both undefined and null.

[dbaeumer]

From https://www.heise.de/developer/artikel/JavaScript-Engines-Performance-Steigerung-der-Browser-4264792.html

fast.js

(() => {
  const han = {firstname: "Han", lastname: "Solo"};
  const luke = {firstname: "Luke", lastname: "Skywalker"};
  const leia = {firstname: "Leia", lastname: "Organa"};
  const obi = {firstname: "Obi-Wan", lastname: "Kenobi"};
  const yoda = {firstname: "", lastname: "Yoda"};

  const people = [
    han, luke, leia, obi,
    yoda, luke, leia, obi
  ];

  const getName = (person) => person.lastname;

  console.time("engine");
  for(var i = 0; i < 1000 * 1000 * 1000; i++) {
    getName(people[i & 7]);
  }
  console.timeEnd("engine");
})();

slow.js

(() => {
  const han = {firstname: "Han", lastname: "Solo", spacecraft: "Falcon"};
  const luke = {firstname: "Luke", lastname: "Skywalker", job: "Jedi"};
  const leia = {firstname: "Leia", lastname: "Organa", gender: "female"};
  const obi = {firstname: "Obi-Wan", lastname: "Kenobi", retired: true};
  const yoda = {lastname: "Yoda"};

  const people = [
    han, luke, leia, obi,
    yoda, luke, leia, obi
  ];

  const getName = (person) => person.lastname;

  console.time("engine");
  for(var i = 0; i < 1000 * 1000 * 1000; i++) {
    getName(people[i & 7]);
  }
  console.timeEnd("engine");
})();

fast-undefined.js

(() => {
  const han = {firstname: "Han", lastname: "Solo", gender: undefined, job: undefined, retired: undefined, spacecraft: "Falcon"};
  const luke = {firstname: "Luke", lastname: "Skywalker", gender: undefined, job: "Jedi", retired: undefined, spacecraft: undefined};
  const leia = {firstname: "Leia", lastname: "Organa", gender: "female", job: undefined, retired: undefined, spacecraft: undefined};
  const obi = {firstname: "Obi-Wan", lastname: "Kenobi", gender: undefined, job: undefined, retired: true, spacecraft: undefined};
  const yoda = {firstname: undefined, lastname: "Yoda", gender: undefined, jon: undefined, retired: undefined, spacecraft: undefined};

  const people = [
    han, luke, leia, obi,
    yoda, luke, leia, obi
  ];

  const getName = (person) => person.lastname;

  console.time("engine");
  for(var i = 0; i < 1000 * 1000 * 1000; i++) {
    getName(people[i & 7]);
  }
  console.timeEnd("engine");
})();

fast-null.js

(() => {
  const han = {firstname: "Han", lastname: "Solo", gender: null, job: null, retired: null, spacecraft: "Falcon"};
  const luke = {firstname: "Luke", lastname: "Skywalker", gender: null, job: "Jedi", retired: null, spacecraft: null};
  const leia = {firstname: "Leia", lastname: "Organa", gender: "female", job: null, retired: null, spacecraft: null};
  const obi = {firstname: "Obi-Wan", lastname: "Kenobi", gender: null, job: null, retired: true, spacecraft: null};
  const yoda = {firstname: null, lastname: "Yoda", gender: null, jon: null, retired: null, spacecraft: null};

  const people = [
    han, luke, leia, obi,
    yoda, luke, leia, obi
  ];

  const getName = (person) => person.lastname;

  console.time("engine");
  for(var i = 0; i < 1000 * 1000 * 1000; i++) {
    getName(people[i & 7]);
  }
  console.timeEnd("engine");
})();

On my machine fast-undefined and fast-null show the same performance. Please note that it is the best to initialize properties with default values matching the type (e.g. '' for string, false for boolean). V8 still deoptimizes when the type of a property changes.

[dbaeumer]

Interestingly the fastest is to have classes with the right initializers:

class.js

(() => {
  class Person {
    constructor({
      firstname = undefined,
      lastname = undefined,
      spaceship = undefined,
      job = undefined,
      gender = undefined,
      retired = undefined
    } = {}) {
      Object.assign(this, {
        firstname,
        lastname,
        spaceship,
        job,
        gender,
        retired
      });
    }
  }
  const han = new Person({
    firstname: "Han",
    lastname: "Solo",
    spaceship: "Falcon"
  });
  const luke = new Person({
    firstname: "Luke",
    lastname: "Skywalker",
    job: "Jedi"
  });
  const leia = new Person({
    firstname: "Leia",
    lastname: "Organa",
    gender: "female"
  });
  const obi = new Person({
    firstname: "Obi-Wan",
    lastname: "Kenobi",
    retired: true
  });
  const yoda = new Person({ lastname: "Yoda" });
  const people = [han, luke, leia, obi, yoda, luke, leia, obi];
  const getName = person => person.lastname;
  console.time("engine");
  for (var i = 0; i < 1000 * 1000 * 1000; i++) {
    getName(people[i & 7]);
  }
  console.timeEnd("engine");
})();

[bpasero]

Everyone decides as they like. I personally will go with "undefined".