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

Refactor/template calc service #2792

Closed
wants to merge 18 commits into from

Conversation

chrismclarke
Copy link
Member

@chrismclarke chrismclarke commented Feb 11, 2025

PR Checklist

  • PR title descriptive (can be used in release notes)

Breaking Changes

The window.calc accessor has been marked as deprecated, e.g. @calc(window.calc.pick_random([1,2,3]))
Any methods available via window.calc.xxxx are already available directly, e.g. @calc(pick_random), so use that way instead.

I can't see any examples of this being used in example_calc, so assume this is mostly a legacy behaviour anyways

The only method that should still be accessed via the window is the date_fns utility library, e.g. @calc(window.date_fns.add_days(...))

Description

Changes merged from #2772 for easier review, a handful of updates to the template-calc.service which handles authored @calc(...) statements, specifically

Author Notes

Beyond the marked deprecation, there shouldn't be any breaking changes or functional testing required.

Review Notes

The code doesn't introduce changes to how calc statements are evaluated (for follow-up), so more just to confirm test cases behave as expected

Dev Notes

The changes introduced here should be mostly non-functional (testing, mock and type-safety improvements). The additional method included is designed for future integration in #2772, so in itself shouldn't be breaking.

Whilst diving into the service I did notice a handful of inconsistencies and clunky areas within the code, however have avoided larger refactor as would be best done through RFC procedures first (beyond marking window.calc as deprecated. Goals of a larger refactor would include:

  • Improve ways to define deployment-specific calc functions
  • Customisable list of 3rd party libs to include in calc (currently just date_fns included for all)
  • Improved parser methods to extract globals required by calc statements and only including those when generating calc context

Git Issues

Closes #

Screenshots/Videos

If useful, provide screenshot or capture to highlight main changes

@chrismclarke
Copy link
Member Author

Rebase into #2793 for cleaner commits

@chrismclarke chrismclarke deleted the refactor/template-calc-service branch February 11, 2025 18:40
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.

1 participant