feat: Add path-aware prefixes instead of numeric suffixes when same-named schemas are found during bundling

lornajane commented 6 months ago

What/Why/How?

Related to #661 , a possible improvement to avoiding name clashes in schema naming.

Check yourself

[ ] Code changed? - Tested with redoc/reference-docs/workflows (internal)
[ ] All new/updated code is covered with tests
[x] New package installed? - Tested in different environments (browser/node)

Security

[x] Security impact of change has been considered
[x] Code follows company security practices and guidelines

changeset-bot[bot] commented 6 months ago

⚠️ No Changeset found

Latest commit: 6444a16b69aaa1ae0d6366620cdc54a3fcf9ffc3

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

github-actions[bot] commented 6 months ago

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`redocly lint packages/core/src/benchmark/benches/rebilly.yaml`	986.7 ± 16.5	949.6	1012.0	1.00 ± 0.02
`redocly-next lint packages/core/src/benchmark/benches/rebilly.yaml`	983.4 ± 17.7	960.4	1008.1	1.00

github-actions[bot] commented 6 months ago

Coverage report

❌ An unexpected error occurred. For more details, check console

Error: The process '/opt/hostedtoolcache/node/20.11.1/x64/bin/npm' failed with exit code 1

St.^{:grey_question:}	Category	Percentage	Covered / Total
🟡	Statements	76.53%	4390/5736
🟡	Branches	66.91%	2358/3524
🟡	Functions	70.17%	722/1029
🟡	Lines	76.7%	4121/5373

Show files with reduced coverage 🔻

|

St.^{:grey_question:}

| File | Statements | Branches | Functions | Lines | | :----------------------------------------------------------------------------------------------------------------------------------------------------: | :----------------- | :------------------------------------------------------------- | :------------------------------------------------------------- | :-------- | :------------------------------------------------------------- | | 🟡 | core/src/bundle.ts |

73.58% (-4.94% 🔻)

|

67.89% (-2.59% 🔻)

| 94.12% |

73.58% (-4.94% 🔻)

|

Test suite run failed

Failed tests: 2/718. Failed suites: 1/102.

``` ● bundle › should bundle external refs and warn for conflicting names expect(received).toHaveLength(expected) Expected length: 1 Received length: 0 Received array: [] 68 | ref: path.join(__dirname, 'fixtures/refs/openapi-with-external-refs-conflicting-names.yaml'), 69 | }); > 70 | expect(problems).toHaveLength(1); | ^ 71 | expect(problems[0].message).toEqual( 72 | `Two schemas are referenced with the same name but different content. Renamed param-b to param-b-2.` 73 | ); at packages/core/src/__tests__/bundle.test.ts:70:22 at fulfilled (packages/core/src/__tests__/bundle.test.ts:5:58) ● bundle › should place referenced schema inline when referenced schema name resolves to original schema name expect(received).toMatchSnapshot() Snapshot name: `bundle should place referenced schema inline when referenced schema name resolves to original schema name 1` - Snapshot - 2 + Received + 4 @@ -26,15 +26,15 @@ simple: type: string A: type: string test: - $ref: '#/components/schemas/rename-2' + $ref: '#/components/schemas/refs-rename' rename: type: string rename-2: - type: number + $ref: '#/components/schemas/refs-rename' vendor.schema: title: vendor type: object description: Vendors properties: @@ -55,6 +55,8 @@ description: Name of the vendor. isOneTimeUse: type: boolean description: One-time use default: false + refs-rename: + type: number ↵ 94 | 95 | expect(problems).toHaveLength(0); > 96 | expect(res.parsed).toMatchSnapshot(); | ^ 97 | }); 98 | 99 | it('should not place referenced schema inline when component in question is not of type "schemas"', async () => { at packages/core/src/__tests__/bundle.test.ts:96:24 at fulfilled (packages/core/src/__tests__/bundle.test.ts:5:58) ```

Report generated by 🧪jest coverage report action from 6444a16b69aaa1ae0d6366620cdc54a3fcf9ffc3

lornajane commented 5 months ago

Proposal not pursued and no longer needed.

Redocly / redocly-cli