Skip to content

Conversation

@Aleksandr-Panin
Copy link
Collaborator

No description provided.

src/index.ts Outdated
import curryToArity from "./util.js";
import curryToArity from "./util";

type Rule = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice to have this is TS. 👍
Maybe as a futre step type could be refined a bit using generics with something like this:

type Rule<Facts, Result, Input> =
  Plainrule<Facts, Result, Input> |
  InjectedRule<Facts, Result, Input> |
  TransformedRule<Facts, Result, Input> |
  FirstRule<Facts, Result, Input> |
  AllRule<Facts, Result, Input> |
  ChainRule<Facts, Result, Input> |
  IfRule<Facts, Result, Input>;

type PlainRule<Facts, Result, Input> = {
  matcher: (facts: Facts, Input: input) => boolean,
  action: (facts: Facts, Input: input) => Result,
}

// ... add types for other rule types here

type RuleResult<Value> = {
  foundMatch: boolean;
  value: Value;
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed it :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome looks good 👍

One more small suggestion: Maybe if possible, instead of defaulting to any in the type signatures like Facts = any we could use Facts = unknown. That way we force the user to pass a type and errors will be caught quicker at build time.

Copy link
Collaborator Author

@Aleksandr-Panin Aleksandr-Panin Apr 15, 2025

Choose a reason for hiding this comment

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

Agree, it needs to be done. Just worry for existing clients - we can break compile step. Suggest as next step :)

@Aleksandr-Panin Aleksandr-Panin merged commit 983f80b into BurdaForward:main Apr 16, 2025
3 checks passed
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.

2 participants