Skip to content

Conversation

@vadyvas
Copy link
Contributor

@vadyvas vadyvas commented Oct 9, 2025

What/Why/How?

Normalize explicit self-file $ref to local internal pointers during bundling and avoid external loading for self-references in resolver.

swagger: '2.0'
info:
  version: '2021-09-01'
  title: Test
paths:
  /test:
    get:
      operationId: Test_Operation
      parameters:
        - $ref: 'test-api.yaml#/parameters/myParam'
parameters:
  myParam:
    name: myParam
    in: query
    type: string

Becomes:

paths:
  /test:
    get:
      parameters:
        - $ref: '#/parameters/myParam'

Reference

fix #1597

Testing

  • Unit tests: added to packages/core/src/__tests__/bundle.test.ts
  • E2E: added __tests__/bundle/self-file-ref-normalization.
  • Existing suites pass locally.

Screenshots (optional)

Check yourself

  • Code changed? - Tested with Redoc/Realm/Reunite (internal)
  • All new/updated code is covered by tests
  • New package installed? - Tested in different environments (browser/node)
  • Documentation update considered

Security

  • The security impact of the change has been considered
  • Code follows company security practices and guidelines

@changeset-bot
Copy link

changeset-bot bot commented Oct 9, 2025

🦋 Changeset detected

Latest commit: 83c545d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@redocly/openapi-core Patch
@redocly/cli Patch
@redocly/respect-core Patch

Not sure what this means? Click here to learn what changesets are.

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

@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2025

Command Mean [s] Min [s] Max [s] Relative
redocly lint packages/core/src/benchmark/benches/rebilly.yaml 1.406 ± 0.017 1.384 1.441 1.00
redocly-next lint packages/core/src/benchmark/benches/rebilly.yaml 1.414 ± 0.032 1.378 1.462 1.01 ± 0.03

@vadyvas vadyvas marked this pull request as ready for review October 9, 2025 14:17
@vadyvas vadyvas requested review from a team as code owners October 9, 2025 14:17
Copy link
Collaborator

@tatomyr tatomyr left a comment

Choose a reason for hiding this comment

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

Left a couple of minor suggestions. Also, could you modify some tests to first point to another file and then use self reference inside of it (just to be sure it works not only for root files)?

Comment on lines 5 to 9
license:
name: MIT
servers:
- url: http://example.com
security: []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please keep the test description minimal to only showcase the issue.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need license, summary, operationId.

Comment on lines 5 to 6
extends:
- recommended
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you need this for?

title: AsyncAPI 2 self-file ref
version: '1.0.0'
channels:
c:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you use the naming consistently here and in other tests (big/small letters; using/not using number)? Preferably something like we already use in our examples (TestChannel/foo/...).

@tatomyr
Copy link
Collaborator

tatomyr commented Oct 10, 2025

One more thing: please make sure to test cases mentioned in this comment.

Copy link
Collaborator

@tatomyr tatomyr left a comment

Choose a reason for hiding this comment

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

Approving with minor suggestions.

Comment on lines 5 to 9
license:
name: MIT
servers:
- url: http://example.com
security: []
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need license, summary, operationId.

}

// Normalize explicit self-file refs to internal pointer
if (ctx.location.source.absoluteRef === resolved.location.source.absoluteRef) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will it still work if we change it this way?

Suggested change
if (ctx.location.source.absoluteRef === resolved.location.source.absoluteRef) {
if (ctx.location.source === resolved.location.source) {

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or the location pointers are actually different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tatomyr moved the link replacement to the check below

!dereference
) {
// Normalize explicit self-file refs to internal pointer
node.$ref = resolved.location.pointer;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please check if comparing those values first will make performance better:

Suggested change
node.$ref = resolved.location.pointer;
if (node.$ref !== resolved.location.pointer) {
node.$ref = resolved.location.pointer;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change improves performance, updated the pr

image

@vadyvas vadyvas merged commit 26e0206 into main Oct 14, 2025
36 checks passed
@vadyvas vadyvas deleted the fix/core-self-file-ref-normalization branch October 14, 2025 10:14
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.

Bundle command does not replace self-referencing files.

4 participants