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 bundling and inlining of referenced schemas #3430

Merged
merged 5 commits into from
May 24, 2024

Conversation

bterlson
Copy link
Member

@bterlson bterlson commented May 23, 2024

Fixes #3369 by changing how types are bundled. In particular, when a type is not a JSON Schema type, we never create a root schema for it. Instead, it is inlined into the defs of any schema which references it, and referenced using a JSON pointer. This PR makes bundling have essentially no impact on emitted schemas, and is merely a way to bundle them into a single file.

The approach is as follows:

  • When a type references another type outside a JSON Schema namespace, include the referenced type under $defs:
    • Such referenced types do not have a $id or $schema field
    • Such referenced types are referenced via JSON pointers not ids
  • Bundling does not alter the bundled schemas or introduce new root schemas. This changes two things from what we do today:
    • The $id of the bundled schemas now includes the file path as it does for non-bundled schemas (whereas before it was just the type name)
    • non-JSON Schema types do not get $defs in the bundle, so the bundle has the same root schemas as would be written to disk when not bundling.
       
      In terms of implementation, the basic approach is to not handle bundling via the emitter framework source files. Instead, we always create source files for root schemas, and inline the necessary defs as we did before (but now using JSON pointers). Then when we're about to write source files, if we're bundling we assemble the bundle and emit that single file, otherwise we emit each source file that contains a root schema.

Todo:

  • Validate that the bundled schemas continue to work with ajv.
  • Cleanups

@azure-sdk
Copy link
Collaborator

azure-sdk commented May 23, 2024

All changed packages have been documented.

  • @typespec/json-schema
Show changes

@typespec/json-schema - fix ✏️

The emitted JSON Schema now doesn't make root schemas for TypeSpec types which do not have the @jsonschema decorator or are contained in a namespace with that decorator. Instead, such schemas are put into the $defs of any root schema which references them, and are referenced using JSON Pointers.

@azure-sdk
Copy link
Collaborator

You can try these changes at https://cadlplayground.z22.web.core.windows.net/prs/3430/

Check the website changes at https://tspwebsitepr.z22.web.core.windows.net/prs/3430/

@bterlson
Copy link
Member Author

@Veetaha if you like you can check the behavior in the playground link above and see if it meets your approval.

docs/emitters/json-schema/reference/index.mdx Outdated Show resolved Hide resolved
docs/emitters/json-schema/reference/index.mdx Outdated Show resolved Hide resolved
docs/emitters/json-schema/reference/index.mdx Outdated Show resolved Hide resolved
packages/json-schema/src/json-schema-emitter.ts Outdated Show resolved Hide resolved
packages/json-schema/test/bundling.test.ts Show resolved Hide resolved
@bterlson bterlson enabled auto-merge May 24, 2024 00:09
@bterlson bterlson added this pull request to the merge queue May 24, 2024
Merged via the queue into microsoft:main with commit 9a10990 May 24, 2024
21 checks passed
@bterlson bterlson deleted the json-schema-reftype-fix branch May 24, 2024 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working emitter:json-schema
Projects
None yet
Development

Successfully merging this pull request may close these issues.

All $refs must use #/$defs/{def_name} syntax rather than {def_name}.yaml/json
4 participants