-
Notifications
You must be signed in to change notification settings - Fork 30
Ensure storylinesContent is marked clearly as nullable in tag page schema
#15118
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
Conversation
| canonicalUrl?: string; | ||
| contributionsServiceUrl: string; | ||
| storylinesContent?: StorylinesContent; | ||
| // This comment is needed to ensure null is recognised properly in the 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 is deliberately not a JSDoc comment to avoid this appearing in the schema itself
| }, | ||
| branding: tagPageBranding, | ||
| canonicalUrl: data.canonicalUrl, | ||
| storylinesContent: data.storylinesContent ?? undefined, |
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.
Use nullish coalescing operator to convert to undefined for rest of DCR to use if null
deedeeh
left a comment
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.
Thank you for fixing that @cemms1 🙏🏼
|
Thanks, hadn't spotted this as an issue when adding the value 😅 |
What does this change?
Adds a nullable type hint comment to the new
storylinesContentfield in the frontend tag page type to ensure the schema treats the field as nullableWhy?
Seems
typescript-json-schemadoesn't recognise nullable types well when translating from types. See YousefED/typescript-json-schema#419Since the addition of
storylinesContent(#15008), I have had problems rendering tag pages locally using DCR due to schema check failuresThis is affecting the Playwright runs in the commercial repo due to it needing to run DCR locally
Screenshots
Example tag page with null
storylinesContent: https://www.theguardian.com/politics/industrial-policy