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

Add autocomplete for operator #392

Open
Ceres6 opened this issue Nov 9, 2024 · 4 comments
Open

Add autocomplete for operator #392

Ceres6 opened this issue Nov 9, 2024 · 4 comments

Comments

@Ceres6
Copy link

Ceres6 commented Nov 9, 2024

To avoid mistakes it would be great to have operator autocomplete in types.

We could either just use a union type and then allow also for any string to account for custom operators, or we could be strict and pair each operator with its corresponding value (e.g. lessThan needs a value: number, and we could document them so users know how to extend that with their custom types.

If there is interest in this I'd be happy to contribute the PR.

Thanks in advance.

@chris-pardy
Copy link
Collaborator

@Ceres6
I think there are 2 potential issues with this.

  1. The availability of custom operators - including custom operators that repurpose the name of existing operators can break any typing. While you can build some very complex typing that makes this work it just raises the level of difficulty of actually using the engine.
  2. I don't think we should encourage anyone to write rules in typescript files where they can be type-checked and auto-completed. Generally speaking if you have typescript available to you then just writing typescript is going to be faster and better.

There's been some requests for a json schema and I would put more emphasis on that. It would allow us to validate the json documents and there are editors that use those schema files to enable things like auto-complete on json files.

@Ceres6
Copy link
Author

Ceres6 commented Nov 12, 2024

Hi @chris-pardy I understand the concerns, maybe some loose autocomplete with the currently available operators would address those concerns and still allow for some autocompletion.

If we do something like
type Operator = 'lessThan' | 'greaterThan' | ... | string & {} we would add autocompletion but still allow any other value and even if the operator is repurposed it will still be a valid operator

WDYT?

PS: I would also be open to contribute to the json schema if the project goes that way

@chris-pardy
Copy link
Collaborator

chris-pardy commented Nov 12, 2024

@Ceres6 my take is that we don't need or even want to improve the experience for people writing json rules in typescript. The rules and conditions structures aren't anywhere near as expressive as typescript so I'd push people to limit use of this system to situations where the rules need to serialized either into a database or over the wire. Or where they need to be safely executed.

I know you can make the argument that it's not getting worse but the more specific the types are the harder it is to change features in the future.

If you'd like to create a JSON schema please open a PR, that would be much appreciated.

@jamiter
Copy link

jamiter commented Nov 20, 2024

@chris-pardy, thanks for looking into this!

There is a use case where generic rules are in code and user/organisation specific rules are in the database. The code ones should, in my opinion, have good type checking.

but the more specific the types are the harder it is to change features in the future

Making changes in the future and updating the types accordingly will help consumers of the package tremendously. They will get type warnings when updating, making migration a lot easier, or at least safer.

But rewriting in TypeScript, like mentioned here #388, would solve most issues, won't it? You can of course choose to still not be specific, even when written in TypeScript... But then TypeScript isn't used to it's full potential IMO.


For the above use case, with partly code and partly database rules, what would you recommend to do? Move all rules to the database? That way the database can validate the types.

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

No branches or pull requests

3 participants