Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve typing information during expression compilation #6925

Conversation

Elmacioro
Copy link
Contributor

Describe your changes

Currently, there is a problem with nodes that utilize presets as sometimes labels are not present and are presented as null.
This happens when we want to use a component or fragment with presets, select some values for them and save the scenario.
If there were any errors during compilation of that node presets will have null labels.

Zrzut ekranu z 2024-09-20 12-42-53

This happens because ProcessDictSubstituor dynamically adds and removes presets' labels.

      if (expr.language == Expression.Language.DictKeyWithLabel && !expr.expression.isBlank) {
        if (isReverse)
          addLabelToDictKeyExpression(process, expr, optionalExpressionTypingInfo, nodeExpressionId)
        else
          removeLabelFromDictKeyExpression(process, expr, nodeExpressionId) // no need to keep label in BE

To add labels at that level we need parameters' types which are assigned during compilation. Unfortunately when there is any custom validators error during node compilation we lose typing information.
in ExperssionCompiler.compileNodeParameters()

    allCompiledParams
      .andThen(allParams => Validations.validateWithCustomValidators(allParams, paramValidatorsMap))
      .combine(redundantMissingValidation.map(_ => List()))

allCompiledParams contain the typing info and is of ValidatedNel type. When Validations.validateWithCustomValidators(allParams, paramValidatorsMap) is Invalid we get these errors without previous typing.

To solve this issue I introduced a change which will still pass typing info further even when there are some custom validator errors. Instead of using Validated which can represent two states: Valid and Invalid, I used Ior which lets us have 3 states: Left, Right and Both. First two are self-explanatory and Both can hold both failure and a success. This way we can preserve the typing info while also preserving errors from custom validation. After testing these changes locally, we preserve the typing info and have correct preset labels even when there were errors during compilation.

Zrzut ekranu z 2024-09-20 13-52-13

Checklist before merge

  • Related issue ID is placed at the beginning of PR title in [brackets] (can be GH issue or Nu Jira issue)
  • Code is cleaned from temporary changes and commented out lines
  • Parts of the code that are not easy to understand are documented in the code
  • Changes are covered by automated tests
  • Showcase in dev-application.conf added to demonstrate the feature
  • Documentation added or updated
  • Added entry in Changelog.md describing the change from the perspective of a public distribution user
  • Added MigrationGuide.md entry in the appropriate subcategory if introducing a breaking change
  • Verify that PR will be squashed during merge

Copy link
Contributor

github-actions bot commented Sep 20, 2024

created: #6933
⚠️ Be careful! Snapshot changes are not necessarily the cause of the error. Check the logs.

@Elmacioro Elmacioro force-pushed the preserve-typing-information-when-invalid-during-custom-validation branch from 4fad9ca to a0d2e86 Compare September 23, 2024 10:54
@github-actions github-actions bot added the docs label Sep 23, 2024
@Elmacioro Elmacioro force-pushed the preserve-typing-information-when-invalid-during-custom-validation branch from a0d2e86 to 03d4a47 Compare September 23, 2024 10:56
@Elmacioro Elmacioro force-pushed the preserve-typing-information-when-invalid-during-custom-validation branch from 8d012c9 to b72a900 Compare September 23, 2024 12:54
@Elmacioro Elmacioro requested a review from arkadius September 23, 2024 13:42
@Elmacioro Elmacioro force-pushed the preserve-typing-information-when-invalid-during-custom-validation branch from b72a900 to 39fe35d Compare September 26, 2024 06:49
Copy link
Member

@arkadius arkadius left a comment

Choose a reason for hiding this comment

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

Good job, one small comment added. Merge it after fix it

@Elmacioro Elmacioro force-pushed the preserve-typing-information-when-invalid-during-custom-validation branch from de319eb to 0b99d56 Compare September 26, 2024 10:00
@Elmacioro Elmacioro merged commit ab4f210 into staging Sep 26, 2024
5 checks passed
@Elmacioro Elmacioro deleted the preserve-typing-information-when-invalid-during-custom-validation branch September 26, 2024 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants