Use the spread operator to conditionally create the settings object

[programarivm]

In JavaScript, it is a quite common thing to conditionally add key-value pairs to objects. This can be done using if statements as in the handleCreateCode() method in the CreateInboxCodeDialog component.

  // ...

  const handleCreateCode = (event) => {
    event.preventDefault();
    let settings = {};
    if (fields.fen) {
      settings.fen = fields.fen;
    }
    if (fields.startPos) {
      settings.startPos = fields.startPos;
    }
    Ws.inboxCreate(fields.variant, JSON.stringify(settings));
  }

  // ...

However, the spread operator allows to do the exact same thing sometimes a little less verbosely.

  // ...

  const handleCreateCode = (event) => {
    event.preventDefault();
    const settings = {
      ...(fields.fen && {fen: fields.fen}),
      ...(fields.startPos && {startPos: fields.startPos})
    };
    Ws.inboxCreate(fields.variant, JSON.stringify(settings));
  }

  // ...

The spread operator should be used in the handleCreateCode() method to create the settings object as described in this issue, which is definitely a good first issue to get to know the codebase.

Keep it up, and happy learning!

[JoeHartzell]

The spread operator in this case is potentially harmful. The spread operator requires us to create an additional object per property. It also requires an iteration per object to apply the keys/values. This could impact performance if there were a lot of settings.

I would propose an alternative approach. JSON.stringify will not include undefined properties in its output. This means you could do something like the following

  const handleCreateCode = (event) => {
    event.preventDefault();
    const settings = {
      fen: fields.fen ?? undefined
      startPos: fields.startPos ?? undefined
    };
    Ws.inboxCreate(fields.variant, JSON.stringify(settings));
  }

This removes the need for spread operator all together and doesn't add any additional objects or iterations. Here is a good article outlining the performance issues with the spread operator (See Here)

[JoeHartzell]

Another potential bug with the code suggested (in the description) is that "falsy" values would not be persisted to the settings. For example if fields.startPos had a value of 0. The line of code below would not persist that to the settings object. This is because the code is using the "truthy/falsy" value instead of being explicit.

// && will be false for things like 0, "", etc...
...(fields.startPos && {startPos: fields.startPos})

My suggested change above uses the null coalescing operator which will only execute for null or undefined values. This is much safer as "truthy" and "falsy" values can still be valid.