Restrict content scope values to string | number | null | undefined - #6295
Draft
VPS-Obi wants to merge 4 commits into
Draft
Restrict content scope values to string | number | null | undefined#6295VPS-Obi wants to merge 4 commits into
VPS-Obi wants to merge 4 commits into
Conversation
The content scope interfaces accepted values of any type (Record<string, any>, [key: string]: unknown), so mistakes involving scopes went unnoticed by the compiler. Narrowing the value type to string | number | null | undefined surfaced several of them: - Embeddable scope properties and resolver scope arguments were typed `typeof Scope`, the scope class, instead of a scope instance. - The Brevo redirect entity factory had the same problem. - DeleteUnsubscribedBrevoContactsConsole wrapped the scope in another object before passing it to the Brevo contacts API, so the API client was resolved for a scope that can never match a Brevo config. Router match params in the admin are always strings, so they no longer derive their type from ContentScope. Scope classes need an explicit index signature to stay assignable to the scope interfaces, as TypeScript does not infer one for classes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnYJ1LdUKw9FxdKdFAdWAr
All scope dimensions in Demo are strings, and an index signature only has to be assignable to the scope interface's, not identical to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnYJ1LdUKw9FxdKdFAdWAr
A project's scope class declares its own index signature, so a scope with only string dimensions keeps string values and needs no conversion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnYJ1LdUKw9FxdKdFAdWAr
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
generate-schema.ts declares its own empty EmailCampaignScope, which still had an unknown index signature and no longer implements EmailCampaignScopeInterface. The file lives outside src/, so neither lint:tsc nor eslint covers it; only the schema generation step in CI compiles it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NnYJ1LdUKw9FxdKdFAdWAr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This change narrows the type of content scope values across multiple packages from
anytostring | number | null | undefined. This prevents mistakes such as passing a scope class instead of a scope instance, or wrapping a scope in another object, by making the type system more restrictive.Key Changes
Type Interface Updates:
ContentScopein@dextinity/cms-admin: Changed from[key: string]: anyto[key: string]: string | number | null | undefinedScopeInterfacein@dextinity/cms-api(page tree): Updated to restrict valuesRedirectScopeInterfacein@dextinity/cms-api: Updated to restrict valuesDamScopeInterfacein@dextinity/cms-api: Updated to restrict valuesEmailCampaignScopeInterfacein@dextinity/brevo-api: Updated to restrict valuesscopein preview params (@dextinity/site-nextjs): Updated to restrict valuesType Corrections in Resolvers and Services:
typeof Scope(class type) toEmailCampaignScopeInterface,DamScopeInterface,RedirectScopeInterface, andScopeInterface(instance types)Internal Type Simplifications:
NonNull<T>andNonNullRecord<T>utility types in favor of simplerContentScopeRouterParamstypenull,undefined, and empty string values consistentlyDemo Application Updates:
[key: string]: stringindex signature to all scope classes (PageTreeNodeScope,DamScope,EmailCampaignContentScope,NewsContentScope,RedirectScope,EmailContactSubscribeScope)Component and Hook Updates:
ContentScopeMigration Notes
Scope classes in applications need to declare an index signature. For scopes with only string dimensions, use
[key: string]: string:https://claude.ai/code/session_01NnYJ1LdUKw9FxdKdFAdWAr