docs icon indicating copy to clipboard operation
docs copied to clipboard

Add clarity to fromJSON function for expressions in Actions

Open bewuethr opened this issue 1 year ago • 2 comments

Code of Conduct

What article on docs.github.com is affected?

GitHub Actions / Learn GitHub Actions / Expressions: https://docs.github.com/en/actions/learn-github-actions/expressions

What part(s) of the article would you like to see updated?

In the section "Functions", under "fromJSON", the section "Example returning a JSON data type" currently looks like this:

This workflow uses fromJSON to convert environment variables from a string to a Boolean or integer.

name: print
on: push
env:
  continue: true
  time: 3
jobs:
  job1:
    runs-on: ubuntu-latest
    steps:
      - continue-on-error: {% raw %}${{ fromJSON(env.continue) }}{% endraw %}
        timeout-minutes: {% raw %}${{ fromJSON(env.time) }}{% endraw %}
        run: echo ...

This example workflow sets environment variables: continue is set to a boolean value true, time is set to an integer value 3.

The workflow uses the fromJSON() function to convert the environment variable continue from a string to a boolean, allowing it to determine whether to continue-on-error or not. Similarly, it converts the time environment variable from a string to an integer, setting the timeout for the job in minutes.

The paragraph right below the code block is confusing, because it implies that continue and time have non-string types, when they're really strings. Both the sentence before the code block and the last sentence make that more clear.

Really, the type of everything set in env is a string, and similar for everything set as a step output. For example, if you set a value like myval='true', with literal quotes, like this

echo "myval='true'" >> "$GITHUB_OUTPUT"

and then use fromJSON like this:

${{ fromJSON(myval) == true }}

the expression evaluates as false, because it's comparing 'true' == true, a string to a boolean.

These are considered true, though:

${{ fromJSON(myval) == 'true' }}
${{ myval == 'true' }}

This can be very confusing when you set a boolean somewhere (but really, it's a string!), and then you have a condition based on that. For example:

      - id: set
        run: |
          echo "myval=false"  >> "$GITHUB_OUTPUT"

      - name: Conditional step
        if: '!steps.set.outputs.myval'
        run: echo "I'd expect this to run"

I'd reasonably expect the conditional step to be run: myval is false, and I use logical NOT, so it should be true. What really happens is that I'm taking the string false, and a string seems to be considered truthy, so NOT a string is false, and the step gets skipped.

To add to the confusion, further up in the same document, at Operators it says that for equality operations, strings are coerced to NaN if they're' not a legal JSON number format, but according to the linked MDN doc, NaN is falsy, so !NaN should be true?

I have created a workflow to test these equalities and better understand them (example run).


tl;dr

  1. The paragraph below the code block in the "Example returning a JSON data type" is confusing, can it be updated to be more clear about environment values always being strings?
  2. Coercions should mention how different types interact with !, because the behaviour seems to contradict the NaN coercion described for equality comparisons.

Additional information

No response

bewuethr avatar Feb 17 '24 22:02 bewuethr

@bewuethr Thank you for opening this issue! I'll get this triaged for review ✨

nguyenalex836 avatar Feb 19 '24 17:02 nguyenalex836

Emeğinize sağlık

gsosm0760 avatar Feb 26 '24 20:02 gsosm0760

Thanks for pointing this out @bewuethr! I agree that both of the sections you highlighted can be clearer.

I think we can close this with two improvements:

  1. Delete the following sentence (it doesn't add anything helpful to the content):

    This example workflow sets environment variables: continue is set to a boolean value true, time is set to an integer value 3.

  2. Change the following sentence:

    from:

    A comparison of one NaN to another NaN does not result in true. For more information, see the "NaN Mozilla docs."

    to:

    When NaN is one of the operands of any relational comparison (>, <, >=, <=), the result is always false. For more information, see the "NaN Mozilla docs."

If you agree with this solution, we welcome you (or anyone else) to open a pull request to make these changes! Check out our contributing documentation for help along the way if needed.

jc-clark avatar Mar 11 '24 18:03 jc-clark

Hi @bewuethr! Thanks for raising this issue, and for providing such helpful documentation. And thanks @jc-clark outlining a very suitable solution. I've made the suggested changes and have opened a PR (#32599) linking this issue. Have a great weekend, everyone 🙌

mark-mxwl avatar Apr 19 '24 22:04 mark-mxwl