lint: several lint error fix

[MaikoTan]

Reported in #271

Some are not actually fixed, but leave a FIXME comment whenever someone are willing to fix it.

[cactbotbot]

@MaikoTan Thanks for your contribution! 🌵🚀

[valarnin]

The linked PR doesn't mention the exact lint errors. What's the error that's being fixed with this change?

[MaikoTan]

\cactbot-workspace\cactbot\ui\raidboss\raidboss_config.ts
   530:43  error  'value' may evaluate to '[object Object]' when stringified     @typescript-eslint/no-base-to-string
   765:26  error  'trigFunc' may evaluate to '[object Object]' when stringified  @typescript-eslint/no-base-to-string
  1349:27  error  'result' may evaluate to '[object Object]' when stringified    @typescript-eslint/no-base-to-string

I am not sure, but ESLint keep complaining that you cannot just .toString() it since the value is possibly an object?

[valarnin]

Alright, I see the issue here. I think this would benefit from a TODO or a FIXME comment, but otherwise the change to JSON.stringify makes sense.

Either this setter func can receive a nested object type as defined by the typedef for SavedConfigEntry, at which point it should handle that nested type properly, or it can't receive a nested type and the typedef for setOptionsFromOutputValue and the underlying type ConfigEntry should have its setterFunc fixed at some point in the future.

[MaikoTan]

Okay, I will add a FIXME comment here and then back to fix it in the future PR.

[valarnin]

The linked PR doesn't mention the exact lint errors. What's the error that's being fixed with this change?

[MaikoTan]

\cactbot\resources\regexes.ts
  324:16  error  Invalid operand for a '+' operation. Operands must each be a number or string, allowing a string + any of: `any`, `boolean`, `null`, `RegExp`, `undefined`. Got `readonly string[]`                                                                                                                  
                                                 @typescript-eslint/restrict-plus-operands
  324:16  error  Invalid operand for a '+' operation. Operands must each be a number or string, allowing a string + any of: `any`, `boolean`, `null`, `RegExp`, `undefined`. Got `RepeatingFieldsMap<T extends "CombatantMemory" ? T : never, (T extends "CombatantMemory" ? T : never) extends "CombatantMemory" ? T extends "CombatantMemory" ? T : never : never>`  @typescript-eslint/restrict-plus-operands

Same as above, but a different error reported.

[valarnin]

Looking at the logic here in its entirety, can fieldName ever be undefined to fall through to this else case? The fieldName local maps to ParseHelperField's field property, which maps back to the fields property in netlog_defs.ts. which doesn't allow undefined as a key.

Could we just drop the entire if/else logic and change it to something like the following (included surrounding lines for context)?

    } else if (fields[keyStr]?.repeating) {
      // If this is a repeating field but the actual value is empty or otherwise invalid,
      // don't process further. We can't use `continue` in the above block because that
      // would skip the early-out break at the end of the loop.
    } else {
      str += Regexes.maybeCapture(
        // more accurate type instead of `as` cast
        // maybe this function needs a refactoring
        capture,
        fieldName,
        fieldValue,
        defaultFieldValue,
      );
    }

    // Stop if we're not capturing and don't care about future fields.
    if (key >= maxKey)
      break;

I haven't pulled the TS 5 upgrade PR down to test it locally yet, but TS 4 doesn't complain about this:

      const testFieldName: string = fieldName;

[MaikoTan]

Hmmm, in this lint fixing PR I don't want to touch too much of the logic, maybe I can open a new PR for adjusting this logic and then back to this PR for the linting error fixing.

And yes, TS 5.0 improved a lot of inference of control flow, it is more sensitive about type errors in if-else blocks.

[valarnin]

If you don't want to adjust logic in this PR that's fine, just add a comment with a TODO or FIXME for this one too? Feel free to link back to this comment thread, if you can't summarize the type narrowing logic in a single line comment.

[MaikoTan]

Okay, I'll do so.

[valarnin]

Pre-approving, pending comments below.

[valarnin]

Looking at the logic here in its entirety, can fieldName ever be undefined to fall through to this else case? The fieldName local maps to ParseHelperField's field property, which maps back to the fields property in netlog_defs.ts. which doesn't allow undefined as a key.

Could we just drop the entire if/else logic and change it to something like the following (included surrounding lines for context)?

    } else if (fields[keyStr]?.repeating) {
      // If this is a repeating field but the actual value is empty or otherwise invalid,
      // don't process further. We can't use `continue` in the above block because that
      // would skip the early-out break at the end of the loop.
    } else {
      str += Regexes.maybeCapture(
        // more accurate type instead of `as` cast
        // maybe this function needs a refactoring
        capture,
        fieldName,
        fieldValue,
        defaultFieldValue,
      );
    }

    // Stop if we're not capturing and don't care about future fields.
    if (key >= maxKey)
      break;

I haven't pulled the TS 5 upgrade PR down to test it locally yet, but TS 4 doesn't complain about this:

      const testFieldName: string = fieldName;

[valarnin]

Alright, I see the issue here. I think this would benefit from a TODO or a FIXME comment, but otherwise the change to JSON.stringify makes sense.

Either this setter func can receive a nested object type as defined by the typedef for SavedConfigEntry, at which point it should handle that nested type properly, or it can't receive a nested type and the typedef for setOptionsFromOutputValue and the underlying type ConfigEntry should have its setterFunc fixed at some point in the future.