Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix invalid $ref index into path #2185

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

duncanbeevers
Copy link
Contributor

@duncanbeevers duncanbeevers commented Feb 28, 2025

⚠️ The Error

Refs into the parameters array are turned into invalid indexes into the paths structure.

🗣️ Discussion

The problematic behavior is triggered using a $ref that points into the parameters array.
The problematic generated type then tries to index into ["parameters"]["0"] as though it were an array, when in fact it should be indexing into ["parameters"]["query"].

This particular weird ref was generated when I bundled a big schema with @apidevtools/json-schema-ref-parser.

🔧 Fix

  • While parsing oapiRefs from parameters arrays, pass the resolved ParameterObject and use its in method to index into parameters type.

This probably doesn't work for more complex $refs, but I think it does slightly improve the semantic awareness of oapiRef.

@duncanbeevers duncanbeevers added bug Something isn't working openapi-ts Relevant to the openapi-typescript library labels Feb 28, 2025
Copy link

netlify bot commented Feb 28, 2025

👷 Deploy request for openapi-ts pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit bd77188

Copy link

changeset-bot bot commented Feb 28, 2025

⚠️ No Changeset found

Latest commit: bd77188

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@gzm0
Copy link
Contributor

gzm0 commented Feb 28, 2025

The problematic generated type then tries to index into ["parameters"]["0"] as though it were an array, when in fact it should be indexing into ["parameters"]["query"].

["parameters"]["path"][0] in the example? (otherwise I need to read up on how the ref "#/paths/~1endpoint/get/parameters/0" is supposed to work 😆 )

@duncanbeevers
Copy link
Contributor Author

duncanbeevers commented Feb 28, 2025

I was also not really familiar with that $ref syntax.

It took a bit of digging, but this portion of the $ref is defined by the JSON Pointer standard.

Evaluation of each reference token begins by decoding any escaped
character sequence. This is performed by first transforming any
occurrence of the sequence '~1' to '/', and then transforming any
occurrence of the sequence '~0' to '~'.

This ref is created during bundling, allowing the same schema to be used throughout the document without first extracting it to components/schemas. Instead, the first instance is inlined, and subsequent uses reference the initial inlined version via a JSON pointer.

However, this JSON Pointer is indexing into the OpenAPI structure, not the openapi-ts paths structure. It seems like there's some oversight in how that JSON Pointer is being transformed between OpenAPI land and openapi-ts land.

{ $ref: "#/components/parameters/query/utm_source" },
{ $ref: "#/components/parameters/query/utm_email" },
{ $ref: "#/components/parameters/query/utm_campaign" },
{ $ref: "#/components/parameters/path/version" },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like these refs were artificially-constructed to match the expected ts output, rather than extracted from a real OpenAPI ref.

Therefore, now that the parameter in is respected, I made them more realistic, and adjusted the custom resolver and expected test output.

@duncanbeevers duncanbeevers force-pushed the intradoc-refs branch 3 times, most recently from d3c7588 to 75281ba Compare March 2, 2025 00:22
@duncanbeevers duncanbeevers marked this pull request as ready for review March 2, 2025 00:23
@duncanbeevers duncanbeevers requested a review from a team as a code owner March 2, 2025 00:23
@duncanbeevers duncanbeevers changed the title Demo bad parameters indexing Fix invalid $ref index into path Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working openapi-ts Relevant to the openapi-typescript library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants