Skip to content

Simplify filter types with a value map and capability union - #2655

Merged
rajarsheechatterjee merged 1 commit into
lnreader:masterfrom
Sleeping-Donut:type-refactor
Oct 10, 2026
Merged

rajarsheechatterjee merged 1 commit into
lnreader:masterfrom
Sleeping-Donut:type-refactor

Conversation

@Sleeping-Donut

Copy link
Copy Markdown
Contributor

Adding a filter type meant restating its value shape across several declarations. Nothing forced the declarations to agree. Omitting a ValueOfFilter branch silently became never, so an incomplete addition still compiled.

Two declarations would now describe a filter type:

  • FilterValueMap is the value shape per type. It's exhaustive, so miss an entry and you get a type error.
  • WithOptions is the types that have an options array. OptionsOf<T> reads from it to add that options array to the resulting filter type.

Things don't get repeated as much. A new FilterType with no options is just two entries in one file — the enum and the value map — and the compiler checks the value map. The refactor leaves the resulting filter types structurally identical to before.
Adding a new capability (the options trait is one) just needs a T extends WithX helper plus a union of the types that support it. So a PR that gives an existing capability to more types only has to add them to the union, which should be easy to review.

Unlike FilterValueMap, WithOptions isn't exhaustive, it only lists the types that have options. If a type is left out — say when adding a new FilterType — it doesn't complain or give a type error, it just treats that type as having no options. So a new FilterType with options has to be remembered, but since it stands out in the list I think that's fine.
Value shapes are still exhaustive, so those can't be half-added.

Enum names/values, exported symbols, and filter literal shapes are unchanged, so no plugin source needs editing. This mirrors the reader-app change (lnreader/lnreader#2093).

I validated that it linted, formatted, and I ran a strict tsc over the repo, this change adds no new errors.

Let me know if it needs changing.

Code assisted by LLM

Checklist

  • Update version code if an existing plugin was modified
  • [ x ] Test changes in Plugin Playground or the app
  • [ x ] Reference related issues in the PR body (e.g. Closes #xyz)
  • [ x ] Commit messages follow type(scope): description (e.g. feat(<generator>): add new source)

Adding a filter type meant restating its value shape across several declarations. Nothing kept them in sync, and a missing `ValueOfFilter` branch silently became `never`.

Consolidate the per-type facts into a value map and a capability union. Each fact is now declared once, and a new capability touches only the types that support it.

`WithOptions` is intentionally a positive list, so a new option-bearing type omitted from it defaults to no options instead of erroring — accepted in exchange for the shorter, easier-to-review list.

Assisted-By: LLM
@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Refactors filter type definitions with a value map.

This PR appears safe to merge; existing filter shapes remain unchanged.

What we checked:

  • Existing filters keep their shapes: The map preserves every existing value shape. WithOptions lists the same three types that previously required options.

Summary

This PR replaces repeated filter declarations with FilterValueMap and WithOptions.

  • Existing filter values, required options, and enum values stay unchanged.
  • Filter now accepts an omitted type argument.
  • No actionable issues were found.

Sleeping-Donut explicitly acknowledged that types omitted from WithOptions have no options as intentional. All current option-bearing types are listed.

Reviews (1) · Last reviewed commit: "refactor: derive filter types from a val..." · Reviewed by Greptile

@rajarsheechatterjee
rajarsheechatterjee merged commit cbca3c7 into lnreader:master Oct 10, 2026
1 check 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