Skip to content

fix(angular): emit filterParams helper for acronym tags in tags-split#3640

Closed
wadakatu wants to merge 1 commit into
orval-labs:masterfrom
wadakatu:fix/3634-angular-tags-split-acronym-filterparams
Closed

fix(angular): emit filterParams helper for acronym tags in tags-split#3640
wadakatu wants to merge 1 commit into
orval-labs:masterfrom
wadakatu:fix/3634-angular-tags-split-acronym-filterparams

Conversation

@wadakatu

@wadakatu wadakatu commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a TS2304: Cannot find name 'filterParams' error in Angular tags-split output for tags whose first word is an acronym (e.g. AB Widget). The generated service calls the shared filterParams helper but never defines or imports it. Single-word tags (Widget) and ordinary multi-word tags (User Account) were unaffected.

Root cause

In tags-split mode each per-tag output file is keyed by kebab(operation.tags[0]) (packages/core/src/writers/target-tags.ts), and that kebab-cased string is passed to the Angular header builder as tag. But getRelevantVerbOptionsForTag — which decides whether the shared filterParams helper is emitted — matched tags with camel:

  • camel('ab-widget')'abWidget' (the incoming kebab-cased tag)
  • camel('AB Widget')'aBWidget' (the operation's original tag — the acronym keeps its second capital)

'abWidget' !== 'aBWidget', so every operation was filtered out, hasBuiltInFilteredQueryParams became false, and the helper was suppressed — while the operation body still called filterParams. camel is not idempotent through a kebab round-trip for acronym tags, which is why only those tags broke.

Fix

Match on kebab instead of camel in getRelevantVerbOptionsForTag so the comparison mirrors the writer's grouping key (kebab(tags[0])). kebab is idempotent (kebab('ab-widget') === 'ab-widget') and DefaultTag is unaffected (kebab('default') === 'default'). This is a single shared function, so the fix covers the httpClient, httpResource, and both Angular modes at once.

Tests

  • New regression spec tests/specifications/issue-3634.yaml with a single-word control tag (Widget) and an acronym trigger tag (AB Widget), wired through a new tags-split Angular config.
  • Focused assertion in tests/api-generation.spec.ts that the acronym-tag service defines filterParams (fails before the fix, passes after).
  • Generated output for all 16 clients type-checks (orval-tests build); full snapshot suite passes with no churn on existing fixtures.

Closes #3634

Summary by CodeRabbit

  • Bug Fixes

    • Fixed tag-scoped output handling in Angular code generation to correctly process multi-word tags with acronym prefixes.
  • Tests

    • Added regression test to verify proper code generation behavior for tag-scoped operations with acronym-based tags.

In tags-split mode the per-tag output file is keyed by `kebab(tags[0])`,
but `getRelevantVerbOptionsForTag` matched tags with `camel`. `camel` is
not idempotent through that round-trip for acronym tags — e.g.
camel('AB Widget') === 'aBWidget' but camel(kebab('AB Widget')) === 'abWidget'
— so every operation was dropped and the shared `filterParams` helper was
suppressed while the operation still called it, producing TS2304.

Match on `kebab` to mirror the writer's grouping key. Adds a regression
spec/test covering an acronym tag alongside a single-word control tag.

Closes orval-labs#3634
Copilot AI review requested due to automatic review settings June 23, 2026 11:02
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8ccd0dd3-df94-46f9-818b-dd5223c21c46

📥 Commits

Reviewing files that changed from the base of the PR and between 9de234d and 143ce20.

⛔ Files ignored due to path filters (6)
  • tests/__snapshots__/angular/issue-3634/ab-widget/ab-widget.service.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/angular/issue-3634/model/index.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/angular/issue-3634/model/listAbWidgetsParams.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/angular/issue-3634/model/listWidgetsParams.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/angular/issue-3634/model/widget.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/angular/issue-3634/widget/widget.service.ts is excluded by !**/__snapshots__/**
📒 Files selected for processing (4)
  • packages/angular/src/utils.ts
  • tests/api-generation.spec.ts
  • tests/configs/angular.config.ts
  • tests/specifications/issue-3634.yaml

📝 Walkthrough

Walkthrough

Fixes a bug where Angular services generated in tags-split mode for tags with acronym prefixes (e.g. AB Widget) were missing the filterParams helper. The fix normalizes tag comparisons in getRelevantVerbOptionsForTag using kebab instead of camel. A new OpenAPI spec, Orval config entry, and Vitest regression test are added to cover this case.

Changes

Angular tags-split filterParams fix (issue-3634)

Layer / File(s) Summary
Tag normalization fix in getRelevantVerbOptionsForTag
packages/angular/src/utils.ts
Replaces the camel import with kebab and updates the tag-matching logic to compare tags via kebab(...) normalization, fixing the missing filterParams for acronym-prefixed tags like AB Widget.
Regression spec, config, and test
tests/specifications/issue-3634.yaml, tests/configs/angular.config.ts, tests/api-generation.spec.ts
Adds a minimal OpenAPI spec with a Widget tag and an AB Widget acronym tag, a new issue3634 Orval config entry with mode: 'tags-split', and a Vitest test asserting filterParams is both defined and invoked in the generated ab-widget.service.ts.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • orval-labs/orval#3115: Introduces kebab(...) tag normalization in packages/query/src/client.ts for the same tag-scoped operation selection pattern that this PR fixes in the Angular counterpart.
  • orval-labs/orval#3359: Modifies getRelevantVerbOptionsForTag to include untagged operations in the default bucket, directly in the same function this PR patches for kebab normalization.

Suggested labels

angular

Suggested reviewers

  • melloware

🐇 A tag named "AB Widget" once tripped on a seam,
Its filterParams lost in a camel-case dream.
Now kebab sets the rules, every dash in its place,
No more broken TypeScript, no more red-faced disgrace.
Hop hop, the bug's fixed — what a glorious day! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the fix: changing tag normalization to use kebab-casing instead of camel-casing for acronym tags in Angular tags-split mode, directly matching the core code change in utils.ts.
Linked Issues check ✅ Passed The PR fully addresses issue #3634 by fixing the tag-matching logic in getRelevantVerbOptionsForTag to use kebab normalization instead of camel, which ensures filterParams helper is emitted for all tags including acronym tags like 'AB Widget'.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing issue #3634: the core fix in utils.ts, the regression test specification, test configuration, and test case file are all necessary and related to resolving the filterParams emission bug for acronym tags.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

Pull request overview

Fixes a bug in Angular tags-split generation where services for acronym-leading tags (e.g. AB Widget) could call filterParams(...) without emitting the shared helper, causing TS2304: Cannot find name 'filterParams'.

Changes:

  • Align tag matching in getRelevantVerbOptionsForTag with the writer’s grouping key by comparing tags via kebab(...) instead of camel(...).
  • Add a regression OpenAPI spec and Angular tags-split test config for issue #3634.
  • Add a focused generation assertion and new snapshots proving filterParams is emitted for the acronym-tag service.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/angular/src/utils.ts Fixes tag filtering by matching tags using kebab(...), ensuring helpers are emitted for the correct per-tag output file.
tests/specifications/issue-3634.yaml Adds a minimal spec reproducing the acronym-tag filterParams emission bug.
tests/configs/angular.config.ts Wires the new issue fixture into the Angular generation test matrix (tags-split).
tests/api-generation.spec.ts Adds a targeted assertion that the acronym-tag service both calls and defines filterParams.
tests/snapshots/angular/issue-3634/** Adds generated output snapshots demonstrating the corrected helper emission.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@the-ult

the-ult commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Heads up @wadakatu. I was working in it as well — we independently converged on the same fix… 😇

Your PR cleanly resolves #3634. I opened #3641 which builds on the same root cause and goes further: generalizes the rule into a single @orval/core source of truth (it's hand-copied across ~6 sites in 4 packages) and fixes the same bug in @orval/hono (raw AB Widget/ dir vs canonical import + empty handler block). Happy to rebase on top of yours once it lands and drop the duplicated Angular bits.

cc: @melloware

@wadakatu

Copy link
Copy Markdown
Contributor Author

@the-ult

Thank you for your PR!
I think that your PR is much cleaner than mine, so I will leave it up to you.

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.

filterParams helper is missing for some tags (tags-split + Angular)

3 participants