HarperFast / HarperFast/skills

Add a rule for migrating legacy instance-verb resources to static methods (loadAsInstance)

Open
#78 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
3
Forks
0
Avg merge
3d 7h
Merged PRs (30d)
8

Description

## The gap

`custom-resources.md` and `extending-tables.md` both correctly prescribe `static` verb methods, and `v5-upgrade.md` says it outright ("Use `static` methods on Resources/Tables to implement endpoints"). What no rule covers is the **other half of that instruction — what to do with a resource class that is already on the legacy instance-verb form.**

`loadAsInstance` does not appear anywhere in this repo:

```
$ grep -rn "loadAsInstance" . --exclude-dir=node_modules --exclude-dir=.git
(no matches)
```

The skills are written for greenfield apps. An agent working in an existing Harper app reads "use static methods", looks at a class carrying `static loadAsInstance = false`, and has no guidance at all.

## Why that is worse than a normal docs gap

The obvious "cleanup" an agent will reach for is deleting the flag. That is a silent breaking change. From the v5 docs ([`reference/resources/resource-api.md`](https://github.com/HarperDB/documentation/blob/main/reference/resources/resource-api.md)):

> The flag does still apply in v5 to the legacy pattern of defining REST-mirroring **instance** verbs (instance `get`, `put`, `patch`, `post`, `delete`, `publish`, `search`), where it selects the argument order those methods receive and whether the record is preloaded onto the instance. Because that behavior is live rather than inert, **`static loadAsInstance = false;` cannot simply be deleted** from an existing resource — dropping the line reverses the argument order for those instance methods and will break them. Remove it only as part of converting them to static methods.

Concretely, in `Resource.ts`:

```js
return resource.constructor.loadAsInstance === false
? resource.post(query, data) // flag set
: resource.post(data, query); // flag absent
```

So `async post(target, data)` and `async post(data, query)` are both valid, look nearly identical, and differ only by a line elsewhere in the class. Deleting that line produces no type error and no test failure unless a test happens to exercise the swapped argument — the handler just starts receiving the wrong object.

## This is not hypothetical

One of our own first-party control-plane Harper apps currently has **49 resource classes** (`extends Resource` and `extends tables.X`), **24** of them carrying `static loadAsInstance = false`, and **zero** static verbs — including classes added this month. Half the repo is on one argument order and half on the other, and nothing in the skills tells an agent that the two halves are different or which one it is looking at. We are having to write that warning into a per-repo `AGENTS.md` because the skill does not carry it.

## Proposed rule: `migrating-instance-verbs-to-static`

Suggested coverage:

1. **How to recognize which form a class is on** — presence/absence of `static loadAsInstance = false`, and the resulting signature for each verb.
2. **The argument-order table**, so the two forms are visually distinguishable:

| `loadAsInstance` | instance verb signature | record preloaded onto `this` |
| --- | --- | --- |
| `false` | `post(target, data)` | no |
| unset | `post(data, query)` | yes |

3. **The load-bearing warning**: never delete the flag on its own. Convert the verbs in the same change or leave it alone.
4. **A worked conversion** — instance → static for both forms, including `super` argument shapes (`extending-tables.md` already has the `super` table; this should reference it rather than duplicate it).
5. **What else changes on the static path.** At minimum: `allow*` hooks are skipped entirely when `loadAsInstance === false`, so an authorization hook on such a class is dead code — a real trap, since it reads as an enforced gate.
6. **Guidance for a partially-migrated repo**: match the file you are editing; conversion is its own reviewed change with its own tests; do not opportunistically convert while fixing something else.

## Related

- #22 (`custom-resources`: target routing, response patterns, error handling) — adjacent surface, different gap.
- #12 (converting existing apps) — related in spirit but explicitly scoped to porting *from other platforms* to Harper, not modernizing an existing Harper app, so this is a sibling rather than a child.

Contributor guide

Open the contributing guide

Research direction

Start by reading custom-resources.md, extending-tables.md, and v5-upgrade.md, then inspect Resource.ts for loadAsInstance behavior and the instance-verb argument shapes. Add a migrating-instance-verbs-to-static rule covering recognition, argument order, conversion, skipped allow hooks, and partially migrated repositories. Done means the rule warns against deleting the flag alone and references the existing super-argument guidance.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, typescript
Domain
backend-api-design, documentation
Issue type
Documentation
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.