-
Notifications
You must be signed in to change notification settings - Fork 27
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
implement versioned docs for OpenAPI w split operation pages #2623
Conversation
The latest updates on your projects. Learn more about Vercel for Git βοΈ
|
π¦ Next.js Bundle AnalysisThis analysis was generated by the next.js bundle analysis action π€ Nine Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. |
c81ce9d
to
a273eeb
Compare
c4e6ebf
to
8d05475
Compare
@@ -30,9 +30,7 @@ import { OpenAPIV3 } from 'openapi-types' | |||
*/ | |||
export async function parseAndValidateOpenApiSchema( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file was already lifted into the shared lib
folder. I think we have a better pattern available for applying schema transforms, established in previous PRs... but previously we were apply the transforms after de-referencing the schema.
With this in mind, I've lightly modified the interface of this function to accommodate an array of transform functions, rather than a single function as was supported previously. I've made corresponding updates in all places where this function is used.
* TODO: this is largely a placeholder for now. | ||
* Will likely require a few more args to pass to getStaticProps, eg productData | ||
* for example, but those types of details are not yet needed by the underlying | ||
* view. | ||
*/ | ||
export default async function getPropsFromPreviewData( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to accommodate versioning, I made updates to the generateStaticProps
function for the new "V2" version of the OpenAPI view. The changes in this file update the preview tool's props generation workflow accordingly.
statusIndicatorConfig?: StatusIndicatorConfig | ||
schemaFileString: string | ||
versionSwitcherSlot?: ReactNode | ||
} | ||
|
||
export function LandingContent({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in this file add support for an optional versionSwitcherSlot
into the landing page content. I also made some arguments optional, to reflect that they're optional in config as well.
@@ -47,17 +48,22 @@ export interface OperationProps { | |||
* Render detailed content for an individual operation. | |||
*/ | |||
export default function OperationContent({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to landing content, the changes in this file add support for a versionSwitcherSlot
in OperationContent
.
// Types | ||
import type { OpenAPIV3 } from 'openapi-types' | ||
import type { OperationContentProps } from '.' | ||
import type { OperationObject } from '../../utils/get-operation-objects' | ||
|
||
/** | ||
* Transform the schemaData into props for an individual operation | ||
*/ | ||
export default async function getOperationContentProps( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made some minor modifications here to avoid passing duplicate data, and to lift the targetOperation
determination process into the enclosing generateStaticProps
.
* Given an array of URL params, parse the context for an OpenAPI v2 URL, and | ||
* Return { isVersionedUrl, versionId, operationSlug } parsed from the URL. | ||
*/ | ||
export function parseOpenApiV2UrlContext( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I split this utility function out since the comments felt worth keeping, but also felt like they were cluttering up the generateStaticPropsVersioned
function body.
* @param operationId | ||
* @returns | ||
*/ | ||
export function slugifyOperationId(operationId: string): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I split this out as it's important for all instances of slugify-ing operation IDs to function in exactly the same way, as they're used to match URLs to the appropriate operation.
* | ||
* Note: only works with `YYYY-MM-DD` version formats. | ||
*/ | ||
export function sortDateVersionData( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied in from the previous version of the view.
const schemaData = await parseAndValidateOpenApiSchema( | ||
schemaFileString, | ||
massageSchemaForClient | ||
) | ||
const schemaData = await parseAndValidateOpenApiSchema(schemaFileString, [ | ||
massageSchemaForClient, | ||
]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note this is the existing open-api-docs-view, not the new "split-operations" one. This change reflects the updated interface for parseAndValidateOpenApiSchema
, which now supports an array of schema transform functions, rather than a single function.
@@ -13,7 +13,6 @@ export { | |||
export { getRequestData } from './get-request-data' | |||
export { getResponseData } from './get-response-data' | |||
export { groupOperations } from './group-operations' | |||
export { parseAndValidateOpenApiSchema } from './parse-and-validate-schema' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This utility function was duplicative of the one at the lib
level. Even though we do plan to delete the entire src/views/open-api-docs-view
directory once migration is complete, it felt right to clean this up now to avoid confusion later.
metadata: { | ||
title: schemaData.info.title, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that our _app.tsx
passes this metadata to our HeadMetadata
component.
const sharedProps: SharedProps = { | ||
basePath, | ||
backToLink, | ||
breadcrumbLinks, | ||
landingLink, | ||
operationLinkGroups, | ||
productData, | ||
product: productData, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our _app.tsx
setup recognizes the product
value on static props, and sets the top nav bar accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good and tests verified βοΈ. Also, nice type interface fixes and excellent comments.
// Construct the label to show in the dropdown | ||
const label = versionId + versionLabelSuffix | ||
// Construct the aria-label for the version dropdown. | ||
const ariaLabel = `Choose a version of the API docs for ${projectName}. Currently viewing version ${label}.` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
β€οΈ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ππ»
5e2eb4a
to
98c1e19
Compare
π Relevant links
ποΈ What
Implements support for versioned OpenAPI docs in our new split-operation-pages
V2
OpenApiDocsView.π€· Why
We want to migrate to a split-operations version of our OpenAPI docs for several reasons. Most significantly:
readme.com
as a demo: https://test-hcp-vs.readme.io/referenceπ οΈ How
Updates
open-api-docs-view-v2
server functions and components to support versioned OpenAPI docs. See PR comments for more detail.πΈ Design Screenshots
See #2624 for a demo of the work done in this PR!
This PR on its own should produce zero visual changes, as we don't make any production changes, and the
OpenApiDocsViewV2
preview tool does not support versioned content.π§ͺ Testing
Validate that the downstream "demo" of the changes in this PR works as expected. See [DO NOT MERGE] add demo migrations to OpenApiDocsViewV2Β #2624 for details, including screenshot comparisons and preview URLs
Validate that the V2 preview tool, at /open-api-docs-preview-v2, still works as expected
.json
format, which can then be used with the preview toolOptionally, to fully validate the changes to
parse-and-validate-open-api-schema.ts
, you could visit existing OpenAPI views. They should match the same views upstream.π Anything else?
Not at the moment!