fix: disambiguate TypeScript navigation metadata for repeated segments - #8247
Shubham Padkonde (Shubham-Padkonde) wants to merge 11 commits into
Conversation
|
Shubham Padkonde (@Shubham-Padkonde) your behaviour is disruptive, opening about 30 PRs on the repos in a couple of hours time, most likely code that was AI generated and not tested/reviewed, will only clog our review pipeline. Please stop opening additional pull requests and work on the comments added to existing ones. We have turned on pull requests limits on this repository, and will proceed to blocking you at the organization level if additional pull requests are opened on other repositories before the already opened ones are reviewed and closed/or merged. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A critical issue remains in navigation metadata resolution for repeated segments.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
Fixes TypeScript metadata name collisions caused by repeated route segments.
Changes:
- Aliases imports that conflict with declarations.
- Resolves navigation metadata from target request-builder files.
- Adds regression tests and changelog coverage.
| File | Summary |
|---|---|
tests/Kiota.Builder.Tests/KiotaBuilderTests.TypeScriptNavigation.cs |
Adds repeated-route navigation regression tests. |
src/Kiota.Builder/Writers/TypeScript/CodeConstantWriter.cs |
Resolves and emits navigation metadata references; a critical lookup issue remains. |
src/Kiota.Builder/Refiners/TypeScriptRefiner.cs |
Adds file-level import aliasing. |
CHANGELOG.md |
Documents the fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Shubham Padkonde (@Shubham-Padkonde) can you solve the conflicts please? |
Okay man, I will try to get merging open PR's and then start working on new contributions. |
please don't, you have over 20 PRs that are still open, let's see those to a resolution first. |
Yes thanks for input, will wait till resolution's for all. Happy to contribute and work with you guys 🫡 |
|
Resolved the changelog conflict in 79b29e3 by merging current main and retaining both entries. The .NET 10 build passes with no warnings; 191 TypeScript tests and all nine repeated-navigation/path-collision generation cases pass. No production changes were added beyond the existing PR and upstream main. This follow-up was prepared and tested with Codex assistance. New Microsoft submissions remain on hold while the existing PRs reach resolution. |
|
Shubham Padkonde (@Shubham-Padkonde) this PR is conflicting, would you mind handling the conflicts when you have a minute please? |
|
This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged. |
|
Conflicts have been resolved. A maintainer will take a look shortly. |
|
This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged. |
|
Conflicts have been resolved. A maintainer will take a look shortly. |
|
This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged. |
|
Conflicts have been resolved. A maintainer will take a look shortly. |
|
This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged. |
|
Conflicts have been resolved. A maintainer will take a look shortly. |
Repeated route segments such as
/api/foo/foo/bargenerate parent and child request builders with the same name. TypeScript aliases the imported interface but leaves the navigation constant unaliased, producing conflicting declarations and a self-reference.Alias imported symbols against all declarations in their code file. Resolve navigation and request metadata from the actual target request builder's file and emit its imported alias, instead of looking up a matching name under the parent namespace.
Fixes #6771.
Validation:
/api/foo,/api/foo/foo,/api/foo/foo/bar,/api/foo/foo/foo/bar, and/api/foo/42/foo/bar. A local stub adapter returns the response; no external HTTP calls are made.Developed and tested with AI assistance. No human review or service-side validation is claimed.