Required inputs undefined. Documentation does not match library.

[Banner-Keith]

Please check our current Issues to see if someone already reported this https://github.com/Microsoft/azure-pipelines-task-lib/issues

Environment

azure-pipelines-task-lib version: 3.2.0

Issue Description

Two issues

1. Documentation out-of-date

The documentation for the input functions is out of date. It shows that they return a string, when they actually return string | undefined

2. Request for improved API functionality

It is very ungainly and difficult to work with this api because we have to handle the undefined case every time we get a required input. The library already throws an error if the input is not provided, so checking again is not necessary.

export function getInput(name: string, required?: boolean): string | undefined {
    var inval = im._vault.retrieveSecret('INPUT_' + im._getVariableKey(name));

    if (required && !inval) {
        throw new Error(loc('LIB_InputRequired', name));
    }

    debug(name + '=' + inval);
    return inval;
}

It would be nice if we could get a version of each of these methods like this so we don't have to check for undefined on every input we pull in. The existing methods would be unchanged so this change would be backwards compatible.

// Name could be getRequiredInput if that is preferrable
export function getInputRequired(name: string): string {
    var inval = im._vault.retrieveSecret('INPUT_' + im._getVariableKey(name));

    if (!inval) {
        throw new Error(loc('LIB_InputRequired', name));
    }

    debug(name + '=' + inval);
    return inval;
}

Example Code

Current behavior:

const exampleInput: string | undefined = tl.getInput('ExampleInput');
const exampleRequiredInput: string | undefined = tl.getInput('ExampleRequiredInput', true);

if (exampleRequiredInput === undefined) {
    throw new Error("Missing ExampleRequiredInput");
}

Desired behavior:

const exampleInput: string | undefined = tl.getInput('ExampleInput');
const exampleRequiredInput: string = tl.getInputRequired('ExampleRequiredInput');

// No checking required because the required input is never undefined

I would be happy to open a Pull Request with these new methods if you would be interested in reviewing my proposal.

[anatolybolshakov]

Hi @Banner-Keith thanks! This could be some possible enhancement to make type validation more straightforward for the case with required value - let us take a look at it in the near time. Feel free to open PR with any of your ideas as well.

[Banner-Keith]

@anatolybolshakov Do you have a preference on the naming convention for the methods? I see either getInputRequired or getRequiredInput as being reasonable names. I kind of prefer putting Required last so that when you start typing 'getInp' IntelliSense suggests both getInput and getInputRequired.

I'll open a PR with my ideas and if you or your team see anything you'd like me to change just let me know.

[max-zaytsev]

@Banner-Keith thanks for doing this! Let us take a look at the pull request.

[max-zaytsev]

This should be available in 3.2.1