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

refactor: link types #367

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/Migration.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as is from "./lib/isValue"
import { getOptionalLinkProperties } from "./lib/getOptionalLinkProperties"
import { validateAssetMetadata } from "./lib/validateAssetMetadata"

import type { Asset } from "./types/api/asset/asset"
Expand Down Expand Up @@ -404,28 +405,32 @@ export class Migration<TDocuments extends PrismicDocument = PrismicDocument> {
*/
#migratePrismicDocumentData(input: unknown): unknown {
if (is.filledContentRelationship(input)) {
const optionalLinkProperties = getOptionalLinkProperties(input)

if (input.isBroken) {
return {
...optionalLinkProperties,
link_type: LinkType.Document,
// ID needs to be 16 characters long to be considered valid by the API
id: "_____broken_____",
isBroken: true,
text: input.text,
}
}

return {
...optionalLinkProperties,
link_type: LinkType.Document,
id: () => this._getByOriginalID(input.id),
text: input.text,
}
}

if (is.filledLinkToMedia(input)) {
const optionalLinkProperties = getOptionalLinkProperties(input)

return {
...optionalLinkProperties,
link_type: LinkType.Media,
id: this.createAsset(input),
text: input.text,
}
}

Expand Down
23 changes: 23 additions & 0 deletions src/lib/getOptionalLinkProperties.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import type { OptionalLinkProperties } from "../types/value/link"

/**
* Returns optional properties only available to link fields. Link fields can
* have the same shape as content relationship and link to media fields,
* requiring special treatment to extract link-specific properties.
*
* @param input - The content relationship or link to media field from which the
* link properties will be extracted.
*
* @returns Optional link properties that `input` might have.
*/
export const getOptionalLinkProperties = (
input: OptionalLinkProperties,
): OptionalLinkProperties => {
const res: OptionalLinkProperties = {}

if ("text" in input) {
res.text = input.text
}

return res
}
13 changes: 11 additions & 2 deletions src/lib/isMigrationValue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
import type { PrismicDocument } from "../types/value/document"
import type { GroupField } from "../types/value/group"
import { LinkType } from "../types/value/link"
import type { OptionalLinkProperties } from "../types/value/link"
import { RichTextNodeType } from "../types/value/richText"
import type { SliceZone } from "../types/value/sliceZone"
import type { AnyRegularField } from "../types/value/types"
Expand All @@ -32,6 +33,10 @@ type UnknownValue =
/**
* Checks if a value is a migration content relationship field.
*
* @remarks
* `OptionalLinkProperties` is included because `MigrationContentRelationship`
* may be a link field, not strictly a content relationship field.
*
* @param value - Value to check.
*
* @returns `true` if `value` is a migration content relationship field, `false`
Expand All @@ -42,7 +47,7 @@ type UnknownValue =
*/
export const contentRelationship = (
value: UnknownValue,
): value is MigrationContentRelationship => {
): value is MigrationContentRelationship & OptionalLinkProperties => {
return (
value instanceof PrismicMigrationDocument ||
is.prismicDocument(value) ||
Expand Down Expand Up @@ -80,6 +85,10 @@ export const image = (value: UnknownValue): value is MigrationImage => {
/**
* Checks if a value is a migration link to media field.
*
* - @remarks `OptionalLinkProperties` is included because
* `MigrationContentRelationship` may be a link field, not strictly a content
* relationship field.
*
* @param value - Value to check.
*
* @returns `true` if `value` is a migration link to media field, `false`
Expand All @@ -90,7 +99,7 @@ export const image = (value: UnknownValue): value is MigrationImage => {
*/
export const linkToMedia = (
value: UnknownValue,
): value is MigrationLinkToMedia => {
): value is MigrationLinkToMedia & OptionalLinkProperties => {
return (
typeof value === "object" &&
value !== null &&
Expand Down
15 changes: 13 additions & 2 deletions src/lib/isValue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { PrismicDocument } from "../types/value/document"
import type { GroupField } from "../types/value/group"
import type { ImageField } from "../types/value/image"
import { LinkType } from "../types/value/link"
import type { OptionalLinkProperties } from "../types/value/link"
import type { FilledLinkToMediaField } from "../types/value/linkToMedia"
import { type RTImageNode, RichTextNodeType } from "../types/value/richText"
import type { SliceZone } from "../types/value/sliceZone"
Expand All @@ -23,6 +24,11 @@ type UnknownValue =
/**
* Checks if a value is a link to media field.
*
* @remarks
* The return value includes `OptionalLinkProperties` because
* `FilledLinkToMediaField` may be a link field, not strictly a content
* relationship field.
*
* @param value - Value to check.
*
* @returns `true` if `value` is a link to media field, `false` otherwise.
Expand All @@ -32,7 +38,7 @@ type UnknownValue =
*/
export const filledLinkToMedia = (
value: UnknownValue,
): value is FilledLinkToMediaField => {
): value is FilledLinkToMediaField & OptionalLinkProperties => {
if (value && typeof value === "object" && !("version" in value)) {
if (
"link_type" in value &&
Expand Down Expand Up @@ -138,6 +144,11 @@ export const rtImageNode = (value: UnknownValue): value is RTImageNode => {
/**
* Checks if a value is a content relationship field.
*
* @remarks
* The return value includes `OptionalLinkProperties` because
* `FilledContentRelationshipField` may be a link field, not strictly a content
* relationship field.
*
* @param value - Value to check.
*
* @returns `true` if `value` is a content relationship, `false` otherwise.
Expand All @@ -147,7 +158,7 @@ export const rtImageNode = (value: UnknownValue): value is RTImageNode => {
*/
export const filledContentRelationship = (
value: UnknownValue,
): value is FilledContentRelationshipField => {
): value is FilledContentRelationshipField & OptionalLinkProperties => {
if (value && typeof value === "object" && !("version" in value)) {
if (
"link_type" in value &&
Expand Down
45 changes: 32 additions & 13 deletions src/lib/resolveMigrationDocumentData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import type {
MigrationContentRelationshipField,
} from "../types/migration/ContentRelationship"
import { PrismicMigrationDocument } from "../types/migration/Document"
import type { MaybeLink } from "../types/migration/Link"
import type { FilledImageFieldImage } from "../types/value/image"
import type { LinkField } from "../types/value/link"
import type { LinkField, OptionalLinkProperties } from "../types/value/link"
import { LinkType } from "../types/value/link"
import type { RTImageNode } from "../types/value/richText"
import { RichTextNodeType } from "../types/value/richText"
Expand All @@ -20,6 +21,7 @@ import * as isFilled from "../helpers/isFilled"
import type { Migration } from "../Migration"

import * as isMigration from "./isMigrationValue"
import { getOptionalLinkProperties } from "./getOptionalLinkProperties"

/**
* Resolves a migration content relationship to a content relationship field.
Expand All @@ -29,35 +31,48 @@ import * as isMigration from "./isMigrationValue"
* @returns Resolved content relationship field.
*/
export async function resolveMigrationContentRelationship(
relation: MigrationContentRelationship,
): Promise<MigrationContentRelationshipField> {
relation: MaybeLink<MigrationContentRelationship>,
): Promise<MigrationContentRelationshipField & OptionalLinkProperties> {
if (typeof relation === "function") {
return resolveMigrationContentRelationship(await relation())
}

if (relation instanceof PrismicMigrationDocument) {
return relation.document.id
? { link_type: LinkType.Document, id: relation.document.id }
: { link_type: LinkType.Document }
: { link_type: LinkType.Any }
}

const optionalLinkProperties =
relation && "link_type" in relation
? getOptionalLinkProperties(relation)
: undefined

if (relation) {
if (
isMigration.contentRelationship(relation.id) ||
typeof relation.id === "function"
typeof relation.id !== "string"
) {
return {
...optionalLinkProperties,
...(await resolveMigrationContentRelationship(relation.id)),
// TODO: Remove when link text PR is merged
// @ts-expect-error - Future-proofing for link text
text: relation.text,
}
}

return { link_type: LinkType.Document, id: relation.id }
// This is only called when resolveMigrationContentRelationship recursively
// calls itself from the statement above and the resolved content relation
// is a Prismic document value.
return {
...optionalLinkProperties,
link_type: LinkType.Document,
id: relation.id,
}
}

return { link_type: LinkType.Document }
return {
...optionalLinkProperties,
link_type: LinkType.Any,
}
}

/**
Expand Down Expand Up @@ -154,20 +169,24 @@ export const resolveMigrationRTImageNode = async (
* @returns Resolved link to media field.
*/
export const resolveMigrationLinkToMedia = (
linkToMedia: MigrationLinkToMedia,
linkToMedia: MaybeLink<MigrationLinkToMedia>,
migration: Migration,
): MigrationLinkToMediaField => {
const asset = migration._assets.get(linkToMedia.id.config.id)?.asset
const optionalLinkProperties = getOptionalLinkProperties(linkToMedia)

if (asset) {
return {
...optionalLinkProperties,
id: asset.id,
link_type: LinkType.Media,
text: linkToMedia.text,
}
}

return { link_type: LinkType.Media }
return {
...optionalLinkProperties,
link_type: LinkType.Any,
}
}

/**
Expand Down
20 changes: 9 additions & 11 deletions src/types/migration/Asset.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { Asset } from "../api/asset/asset"
import type { FilledImageFieldImage } from "../value/image"
import type { EmptyLinkField } from "../value/link"
import type { LinkToMediaField } from "../value/linkToMedia"
import { type RTImageNode } from "../value/richText"

Expand Down Expand Up @@ -73,23 +72,22 @@ export type MigrationImage =
*/
export type MigrationLinkToMedia = Pick<
LinkToMediaField<"filled">,
"link_type"
> &
Partial<Pick<LinkToMediaField<"filled">, "text">> & {
/**
* A reference to the migration asset used to resolve the link to media
* field's value.
*/
id: PrismicMigrationAsset
}
"link_type" | "text"
> & {
/**
* A reference to the migration asset used to resolve the link to media
* field's value.
*/
id: PrismicMigrationAsset
}

/**
* The minimum amount of information needed to represent a link to media field
* with the migration API.
*/
export type MigrationLinkToMediaField =
| Pick<LinkToMediaField<"filled">, "link_type" | "id" | "text">
| EmptyLinkField<"Media">
| LinkToMediaField<"empty">

/**
* A rich text image node in a migration.
Expand Down
20 changes: 11 additions & 9 deletions src/types/migration/ContentRelationship.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import type { FilledContentRelationshipField } from "../value/contentRelationship"
import type {
ContentRelationshipField,
EmptyContentRelationshipField,
FilledContentRelationshipField,
} from "../value/contentRelationship"
import type { PrismicDocument } from "../value/document"
import type { EmptyLinkField } from "../value/link"

import type { PrismicMigrationDocument } from "./Document"

Expand All @@ -13,17 +16,16 @@ export type MigrationContentRelationship<
TDocuments extends PrismicDocument = PrismicDocument,
> =
| ValueOrThunk<TDocuments | PrismicMigrationDocument<TDocuments> | undefined>
| (Pick<FilledContentRelationshipField, "link_type"> &
Partial<Pick<FilledContentRelationshipField, "text">> & {
id: ValueOrThunk<
TDocuments | PrismicMigrationDocument<TDocuments> | undefined
>
})
| (Pick<ContentRelationshipField, "link_type"> & {
id: ValueOrThunk<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Why did you remove text here? It's used in resolveMigrationContentRelationship and from what I understood with Lucie will still be used while handling link to document that can be from a generic link.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

text was removed because this type defines how users specify a content relationship value. text is invalid for content relationship fields.

Internally, we needed to widen the types after verifying the shape, not before. You can see the & OptionalLinkProperties in src/lib/isValue.ts and src/lib/isMigrationValue.ts.

These changes support the following behavior when migrating a document:

  • Content relationship fields will not include text or variant.
  • Link to media fields will not include variant. It includes text only because we explicitly added it to the field type. We can easily remove it for this field type only if we decide to do so.
  • Link fields include text and variant.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, I'm not sure to get everything.

Content relationship fields will not include text or variant.

That I understand, but what's the difference with link to document? What I understood from Lucie is that the type is used also for migrating link to document implying that text and variant are supported.
There is no difference between link to document and content relationship behind the scene. Right?

Link to media fields will not include variant. It includes text only because we explicitly added it to the field type. We can easily remove it for this field type only if we decide to do so.

For me, it's either both text + variant or nothing. We are waiting for the product but meanwhile, I would go for both. (details we can take care of course)

Copy link
Member Author

@angeloashmore angeloashmore Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That I understand, but what's the difference with link to document? What I understood from Lucie is that the type is used also for migrating link to document implying that text and variant are supported.
There is no difference between link to document and content relationship behind the scene. Right?

Content relationship functions are used for both content relationship fields and link fields. From the public API, we want to separate these field types so they only accept what is allowed. From the internal API, we need to treat both field types as overlapping.

Before this PR, we made the overlap public, which meant devs could assign text to a content relationship field, which should not be allowed.

Here is what the autocomplete looks like after this PR for each field type:

Link type Autocomplete Notes
Link field image Includes text and variant, along with the other migration-specific properties.
Link to media image Includes text.
Content relationship image Does not include text or variants.

For me, it's either both text + variant or nothing. We are waiting for the product but meanwhile, I would go for both. (details we can take care of course)

I'm happy to remove text from link to media if we want to do that. I left it in since that's how the product works today. Are you good with removing it from @prismicio/client now?

TDocuments | PrismicMigrationDocument<TDocuments> | undefined
>
})

/**
* The minimum amount of information needed to represent a content relationship
* field with the migration API.
*/
export type MigrationContentRelationshipField =
| Pick<FilledContentRelationshipField, "link_type" | "id">
| EmptyLinkField<"Document">
| EmptyContentRelationshipField
19 changes: 19 additions & 0 deletions src/types/migration/Link.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import type { LinkType, OptionalLinkProperties } from "../value/link"

/**
* Adds `OptionalLinkProperties` to any type that looks like a link object (one
* that includes a valid `link_type` property).
*
* @example
*
* ```ts
* type Example = MaybeLink<PrismicDocument | LinkField>
* // PrismicDocument | (LinkField & OptionalLinkProperties)
* ```
*
* @typeParam T - The type to augment.
*/
export type MaybeLink<T> =
| Exclude<T, { link_type: (typeof LinkType)[keyof typeof LinkType] }>
| (Extract<T, { link_type: (typeof LinkType)[keyof typeof LinkType] }> &
OptionalLinkProperties)
1 change: 0 additions & 1 deletion src/types/model/contentRelationship.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,5 @@ export interface CustomTypeModelContentRelationshipField<
select: typeof CustomTypeModelLinkSelectType.Document
customtypes?: readonly CustomTypeIDs[]
tags?: readonly Tags[]
allowText?: boolean
}
}
Loading
Loading