Upgrade ts-proto with fix for noImplicitReturns.

[absoludity]

Signed-off-by: Michael Nelson minelson@vmware.com

Description of the change

Replaces #3906 . The current version of ts-proto which we use generates typescript that fails our typescript lint checks. This means that any new PR which needs to generate, fails to pass CI.

cc @antgamdia I'm not 100%, but it looks like you may have manually updated the generated content in this commit so that it would pass? I thought of doing the same but we need to be able to work with the generated content.

I've created an issue upstream to see if it's something that may be fixed upstream, in stephenh/ts-proto#432

For now I've just disabled the noImplicitReturns check. Longer term we should re-enable this and also generate these files during our build process so that this is caught immediately when the ts-proto package was upgraded (see #3908)

Benefits

We can continue to modify protobuf files without breaking the build.

Possible drawbacks

Applicable issues

• ref Replace remaining Dashboard calls to K8s API server #3896

Additional information

[boukeversteegh]

Hey @absoludity , thanks for opening an issue in ts-proto, we will fix the issue there.

In the meantime, I thought the following might be helpful. When generated code doesn't match your project's linting rules, you could also disable linting on the generated code, or exclude a violated rule for a particular folder or file. That way you won't be blocked and don't need to disable the rule for your whole project. It's also handy for cases where the rules of the two projects contradict each other, and it cannot be fixed upstream.

For example, you could do this:

// .eslintrc.js
module.exports = {
  // exclude entire directory from linting
  ignorePatterns: ['src/lib/**'],
  overrides: [
    // disable specific rule in directory
    {
      files: ['somedirectory/**/*.ts'],
      rules: {
        '@typescript-eslint/no-undef': 'off',
      },
    },
  ],
}

I'm not an expert on eslint, but this configuration worked for me.

[boukeversteegh]

PR ready stephenh/ts-proto#436

[absoludity]

Excellent, thanks @boukeversteegh ... now I can just update ts-proto instead :) And thanks for the info about the eslintrc..., I did briefly look for a way to suppress it for a particular file in the tsconfig, but missed the overrides in .eslintrc.

[absoludity]

Rubber-stamping this one to unblock changes to proto files.

[castelblanque]

@absoludity After this change (containing the upgrade of ts-proto): could we get rid of the generated TS source code for Dashboard?

[absoludity]

@absoludity After this change (containing the upgrade of ts-proto): could we get rid of the generated TS source code for Dashboard?

It's unrelated to this change/upgrade. We may want to do so but that'll be part of the discussion at #3908 .

[antgamdia]

Sorry, I've been OOO for a while.

cc antgamdia I'm not 100%, but it looks like you may have manually updated the generated content in this commit so that it would pass? I thought of doing the same but we need to be able to work with the generated content.

Yep, you're totally right. I added a return undefined to pass the linter issue (albeit being consistent with the method's signature) after the ts-proto dep upgrade while developing the service account thing. I forgot to look into the issue in-depth and I totally miss that change when committing and sending the PR. My bad. I'm sorry :(

Thanks for catching the issue, Michael, and special thanks to the ts-proto team; what a quick solution!!