Skip to content

refactor: register v1-alpha development routes by directory - #2241

Merged
epipav merged 3 commits into
mainfrom
refactor/IN-1348-autoload-routes
Sep 21, 2026
Merged

epipav merged 3 commits into
mainfrom
refactor/IN-1348-autoload-routes

Conversation

@epipav

@epipav epipav commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Why

Every Development endpoint PR (IN-1331 to IN-1335) had to add an import and a scope.register line to api/src/versions/v1-alpha/index.ts. With five PRs open at once, each merge put the other four into conflict and forced a rebase round. Nine more Development endpoints are still to come, plus the later groups.

What

  • api/src/versions/v1-alpha/index.ts loads every module under development/ with @fastify/autoload (dirNameRoutePrefix: false because route paths are absolute, forceESM: true). An endpoint PR now adds one route file and one test file and touches nothing shared, so merge order stops mattering.
  • api/vitest.config.ts inlines @fastify/autoload (server.deps.inline). Vitest transforms .ts imports only in project code; autoload's dynamic import runs inside node_modules and would otherwise hit Node's "Unknown file extension .ts".
  • api/tests/v1-alpha-autoload.test.ts pins the convention: the modules in development/ and the /v1-alpha/projects/{slug}/development/* paths in the alpha spec match one to one, filename = path leaf. A module that fails to load or registers outside the group now fails a test instead of silently dropping out of the spec.
  • New runtime dependency: @fastify/autoload 6.5.0 (official Fastify plugin, alongside @fastify/static and @fastify/swagger).

Verification

Route tree identical before and after under all three runtimes:

  • vitest: 14 files / 196 tests green, plus tsc-check, lint, format:check
  • tsx dev server: /v1-alpha/projects/:slug/development/issues-resolution registered
  • compiled dist/ under plain Node: same

Adding an endpoint from now on

Create api/src/versions/v1-alpha/development/<path-leaf>.ts with a default-exported Fastify plugin that registers /projects/:slug/development/<path-leaf>, and its test under api/tests/. Nothing else.

Follow-up

The four open endpoint PRs (#2238, #2240, #2239, #2237) are rebased onto this branch so their index.ts edits disappear; they retarget to main when this merges.

Jira: IN-1348

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The autoload behavior is covered by integration testing, preserves existing routing, and passes all completed checks.

Review effort: Balanced
Findings: None

What changed in this PR

Refactors v1-alpha Development route registration to use directory-based Fastify autoloading, reducing merge conflicts for upcoming endpoints.

Changes:

  • Autoloads Development route modules with @fastify/autoload.
  • Configures Vitest to transform dynamically imported TypeScript modules.
  • Tests that route filenames match their OpenAPI path leaves.
File Description
api/​package.json Adds the autoload runtime dependency.
api/​src/​versions/​v1-alpha/​index.ts Registers Development routes by directory.
api/​tests/​v1-alpha-autoload.test.ts Verifies module-to-route consistency.
api/​vitest.config.ts Inlines autoload for TypeScript transformation.
pnpm-lock.yaml Locks the new dependency.
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@gaspergrom gaspergrom left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I checked the autoload config against the manual registration it replaces, and traced through the new test to confirm it asserts a real invariant (filename to route-path-leaf), not something trivially true. Old import and register call are gone. None of the dependent PR's route files show up in this diff. LGTM.

Copilot AI review requested due to automatic review settings September 21, 2026 13:01

@themarolt themarolt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm - one question on what happens when a non-route file lands in development/

Comment thread api/src/versions/v1-alpha/index.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The focused refactor preserves route behavior and adds appropriate convention coverage.

Review effort: Balanced
Findings: None

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

Copilot AI review requested due to automatic review settings September 21, 2026 13:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The convention test can miss routes containing unexpected intermediate path segments.

Review effort: Balanced
Findings: None

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Previously missed (1)

In code that hasn't changed since last review

Medium severity Path validation ignores extra directory segments

api/​tests/​v1-alpha-autoload.test.ts:36

Stripping everything before the final slash lets /development/extra/<filename> pass despite the documented exact-path convention. Compare complete paths derived from each filename instead.

@epipav
epipav merged commit dd37a7a into main Sep 21, 2026
11 checks passed
@epipav
epipav deleted the refactor/IN-1348-autoload-routes branch September 21, 2026 14:16
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.

4 participants