Skip to content

Conversation

@billzdon
Copy link
Contributor

@billzdon billzdon commented Apr 5, 2024

Adds presence ts code gen

Makes params optional if there is a check for presence. Will use "string | boolean | number" type if there are no comparison values or grab the type from the usedVariables if it ever was compared to a value.

If there are any presence checks, we'll use optional value safe methods on strings

Screenshot 2024-04-15 at 4 01 08 PM

case rulesv1beta3.ComparisonOperator_COMPARISON_OPERATOR_ENDS_WITH:
usedVariables[v.Atom.ContextKey] = structpbValueToKindString(v.Atom.ComparisonValue)
return fmt.Sprintf("(%s.endsWith(%s))", v.Atom.ContextKey, try.To1(marshalOptions.Marshal(v.Atom.ComparisonValue)))
case rulesv1beta3.ComparisonOperator_COMPARISON_OPERATOR_PRESENT:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this sufficient?

Like what if I have:

if a.contains("foo")
  return false
if a !== undefined
  return true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you are saying we should always move the undefined check first? Or that logic above should not be allowed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm saying that in that particular situation, "a" should not be optional

@billzdon billzdon requested a review from lekko-jonathan April 15, 2024 16:51
Copy link
Contributor

@lekko-jonathan lekko-jonathan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants