Stronger typings and simpler variable parsing #76
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is based on some experimenting I did in the atem module bitfocus/companion-module-bmd-atem#254
I am very interested in feedback/input from others on this.
Although there is no technical reason for it, I would like to propose making using these 'strict' types mandatory to support the use of location based variables in module actions and feedbacks. This is because it will help ensure that they are handled correctly, and mean that we have control over how the parsing is done, which will help minimise any impact to modules if we need to make future changes. Either way, modules will need to make changes to support location based variables.
The aim is to provide an action and feedback definitions which are strongly typed (to benefit modules using typescipt), and at the same time try to abstract away the parsing of variables.
This has intentionally been done as a 'strict' version, without touching the existing types, so that modules can choose which version they use. This is to avoid it being a breaking change, and allow modules to migrate gradually if they want to, and to do so gradually.
Maybe in the future we will remove the looser flow, but unless it becomes incredibly messy to support both it likely won't be worth the effort.
As an example, some simple actions which use these types: https://github.com/bitfocus/companion-module-bmd-atem/blob/main/src/actions/aux.ts
And example feedbacks: https://github.com/bitfocus/companion-module-bmd-atem/blob/main/src/feedback/aux.ts
And presets: https://github.com/bitfocus/companion-module-bmd-atem/blob/main/src/presets/aux.ts
TODO:
StrictOptionsImpl
should cache the values it has fetched/validated, so that modules don't need to think about whether they might be parsing the same string multiple times.StrictOptions
won't play nicely if we want to allow any supportuseVariables: true
on number input fields. Perhaps all of the getters should be async so that we can allow this?