Skip to content

feat: ext 6788 asset sidebar location constants [EXT-6786] #2338

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Sep 24, 2025
+89 −1
Merged

feat: ext 6788 asset sidebar location constants [EXT-6786] #2338

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e89780e
feat: add ASSET_SIDEBAR location constants
tylerwashington888 Sep 23, 2025
5c43347
feat: implement asset API integration
tylerwashington888 Sep 24, 2025
8346f33
refactor: remove deprecated metadata from AssetAPI interface
tylerwashington888 Sep 24, 2025
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
8 changes: 8 additions & 0 deletions lib/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { makeField } from './field'
import { makeFieldLocale } from './field-locale'
import createWindow from './window'
import createEntry from './entry'
import createAsset from './asset'
import createSpace from './space'
import createDialogs from './dialogs'
import createEditor from './editor'
Expand Down Expand Up @@ -38,6 +39,7 @@ const LOCATION_TO_API_PRODUCERS: { [location: string]: ProducerFunc[] } = {
[locations.LOCATION_ENTRY_FIELD]: DEFAULT_API_PRODUCERS,
[locations.LOCATION_ENTRY_FIELD_SIDEBAR]: DEFAULT_API_PRODUCERS,
[locations.LOCATION_ENTRY_SIDEBAR]: [makeSharedAPI, makeEntryAPI, makeEditorAPI, makeWindowAPI],
[locations.LOCATION_ASSET_SIDEBAR]: [makeSharedAPI, makeAssetAPI, makeWindowAPI],
[locations.LOCATION_ENTRY_EDITOR]: [makeSharedAPI, makeEntryAPI, makeEditorAPI],
[locations.LOCATION_DIALOG]: [makeSharedAPI, makeDialogAPI, makeWindowAPI],
[locations.LOCATION_PAGE]: [makeSharedAPI],
Expand Down Expand Up @@ -151,6 +153,12 @@ function makeDialogAPI(channel: Channel) {
}
}

function makeAssetAPI(channel: Channel, { asset }: ConnectMessage) {
return {
asset: createAsset(channel, asset),
}
}

function makeAppAPI(channel: Channel) {
const app = createApp(channel)

Expand Down
54 changes: 54 additions & 0 deletions lib/asset.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { Channel } from './channel'
import { MemoizedSignal } from './signal'
import { ConnectMessage, Metadata } from './types'
import { AssetAPI } from './types/asset.types'
import { AssetSys } from './types/utils'

export default function createAsset(
channel: Channel,
assetData: ConnectMessage['asset'],
): AssetAPI {
if (!assetData) {
throw new Error('Asset data is required')
}

let sys: AssetSys = assetData.sys
const sysChanged = new MemoizedSignal<[AssetSys]>(sys)
let metadata = assetData.metadata
const metadataChanged = new MemoizedSignal<[Metadata | undefined]>(metadata)

channel.addHandler('sysChanged', (newSys: AssetSys) => {
sys = newSys
sysChanged.dispatch(sys)
})

channel.addHandler('metadataChanged', (newMetadata: Metadata) => {
metadata = newMetadata
metadataChanged.dispatch(metadata)
})

return {
getSys() {
return sys
},
publish(options?: { skipUiValidation?: boolean }) {
return channel.call<void>('callAssetMethod', 'publish', [options])
},
unpublish() {
return channel.call<void>('callAssetMethod', 'unpublish')
},
save() {
return channel.call<void>('callAssetMethod', 'save')
},
onSysChanged(handler: (sys: AssetSys) => void) {
return sysChanged.attach(handler)
},
...(metadata ? { metadata } : {}),
getMetadata() {
return metadata
},
onMetadataChanged(handler: VoidFunction) {
return metadataChanged.attach(handler)
},
Comment on lines +49 to +51
Copy link

@coderabbitai coderabbitai bot Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

Type mismatch: onMetadataChanged callback should receive metadata.

The signature is VoidFunction but listeners are invoked with metadata. Align with AssetAPI type.

Apply this diff:

-    onMetadataChanged(handler: VoidFunction) {
+    onMetadataChanged(handler: (metadata?: Metadata) => void) {
       return metadataChanged.attach(handler)
     },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
onMetadataChanged(handler: VoidFunction) {
return metadataChanged.attach(handler)
},
onMetadataChanged(handler: (metadata?: Metadata) => void) {
return metadataChanged.attach(handler)
},
🤖 Prompt for AI Agents 
lib/asset.ts around lines 50 to 52: the onMetadataChanged handler is currently
typed as VoidFunction but listeners are invoked with metadata; change the
handler signature to accept the metadata parameter matching AssetAPI (e.g.
handler: (metadata: AssetMetadata) => void or the exact type from AssetAPI),
update the function signature and return type accordingly, and import or
reference the correct AssetAPI/Metadata type so attached listeners receive the
typed metadata.

Copy link
Contributor Author

@tylerwashington888 tylerwashington888 Sep 24, 2025

Choose a reason for hiding this comment

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

the identical entry api method uses the same args so im going to keep this the same

Copy link

@coderabbitai coderabbitai bot Sep 24, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for onMetadataChanged method in entry-related files to verify the user's claim about consistency
ast-grep --pattern 'onMetadataChanged($_) {
  $$$
}'

# Also search with rg to find any onMetadataChanged implementations
rg -nC3 "onMetadataChanged" --type=ts

Length of output: 1234

@tylerwashington888 You're absolutely right! I can see that the entry API implementation in lib/entry.ts line 77 uses the exact same pattern:

onMetadataChanged(handler: VoidFunction) {
  return metadataChanged.attach(handler)
}

Both implementations are consistent with each other, even though they differ from their respective type definitions (which correctly specify (metadata?: Metadata) => void). I understand the rationale for maintaining consistency across the codebase. Thanks for the clarification!

✏ Learnings added 
Learnt from: tylerwashington888
PR: contentful/ui-extensions-sdk#2338
File: lib/asset.ts:49-51
Timestamp: 2025-09-24T16:31:58.939Z
Learning: Both asset API and entry API implementations use VoidFunction for onMetadataChanged handler parameter, maintaining consistency across the codebase despite type definitions specifying (metadata?: Metadata) => void.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

}
}
1 change: 1 addition & 0 deletions lib/locations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const locations: Locations = {
LOCATION_ENTRY_FIELD: 'entry-field',
LOCATION_ENTRY_FIELD_SIDEBAR: 'entry-field-sidebar',
LOCATION_ENTRY_SIDEBAR: 'entry-sidebar',
LOCATION_ASSET_SIDEBAR: 'asset-sidebar',
LOCATION_DIALOG: 'dialog',
LOCATION_ENTRY_EDITOR: 'entry-editor',
LOCATION_PAGE: 'page',
Expand Down
7 changes: 6 additions & 1 deletion lib/types/api.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
import { EntryAPI } from './entry.types'
import { SpaceAPI } from './space.types'
import { WindowAPI } from './window.types'
import { EntrySys, Link, SerializedJSONValue } from './utils'
import { EntrySys, AssetSys, Link, SerializedJSONValue } from './utils'
import { FieldAPI } from './field-locale.types'
import { DialogsAPI } from './dialogs.types'
import { AppConfigAPI } from './app.types'
Expand Down Expand Up @@ -386,6 +386,7 @@ export interface Locations {
LOCATION_ENTRY_FIELD: 'entry-field'
LOCATION_ENTRY_FIELD_SIDEBAR: 'entry-field-sidebar'
LOCATION_ENTRY_SIDEBAR: 'entry-sidebar'
LOCATION_ASSET_SIDEBAR: 'asset-sidebar'
LOCATION_DIALOG: 'dialog'
LOCATION_ENTRY_EDITOR: 'entry-editor'
LOCATION_PAGE: 'page'
Expand All @@ -411,6 +412,10 @@ export interface ConnectMessage {
sys: EntrySys
metadata?: Metadata
}
asset?: {
sys: AssetSys
metadata?: Metadata
}
fieldInfo: EntryFieldInfo[]
field?: FieldInfo
hostnames: HostnamesAPI
Expand Down
22 changes: 22 additions & 0 deletions lib/types/asset.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { Metadata } from './entities'
import { AssetSys } from './utils'

export interface AssetAPI {
/** Returns sys for an asset. */
getSys: () => AssetSys
/** Publish the asset */
publish: (options?: { skipUiValidation?: boolean }) => Promise<void>
/** Unpublish the asset */
unpublish: () => Promise<void>
/** Saves the current changes of the asset */
save: () => Promise<void>
/** Calls the callback with sys every time that sys changes. */
onSysChanged: (callback: (sys: AssetSys) => void) => () => void
/**
* Optional metadata on an asset
* @deprecated
*/
metadata?: Metadata
getMetadata: () => Metadata | undefined
onMetadataChanged: (callback: (metadata?: Metadata) => void) => VoidFunction
}
4 changes: 4 additions & 0 deletions lib/types/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ export interface EntrySys extends ContentEntitySys {
automationTags: Link<'Tag'>[]
}

export interface AssetSys extends ContentEntitySys {
type: 'Asset'
}
Comment on lines +73 to +75
Copy link

@coderabbitai coderabbitai bot Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

AssetSys should not inherit contentType (assets don’t have one).

Current inheritance from ContentEntitySys incorrectly forces a contentType link on assets. Define AssetSys by omitting contentType.

Apply this diff:

-export interface AssetSys extends ContentEntitySys {
-  type: 'Asset'
-}
+export type AssetSys = Omit<ContentEntitySys, 'contentType'> & {
+  type: 'Asset'
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export interface AssetSys extends ContentEntitySys {
type: 'Asset'
}
export type AssetSys = Omit<ContentEntitySys, 'contentType'> & {
type: 'Asset'
}
🤖 Prompt for AI Agents 
In lib/types/utils.ts around lines 73 to 75, AssetSys currently inherits
ContentEntitySys which forces a contentType link that assets don't have; change
AssetSys to extend ContentEntitySys with contentType omitted (e.g., use
Omit<ContentEntitySys, 'contentType'>) and keep the type literal 'Asset' so the
interface no longer requires a contentType property.

Copy link
Contributor Author

@tylerwashington888 tylerwashington888 Sep 24, 2025

Choose a reason for hiding this comment

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

the identical entry interface uses the same structure so im going to keep this the same

Copy link

@coderabbitai coderabbitai bot Sep 24, 2025

Choose a reason for hiding this comment

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

@tylerwashington888 Thanks for the clarification! You're absolutely right that maintaining consistency between EntrySys and AssetSys by having them both extend ContentEntitySys is a valid design choice. I understand you want to keep the interface structure aligned across entity types.

✏ Learnings added 
Learnt from: tylerwashington888
PR: contentful/ui-extensions-sdk#2338
File: lib/types/utils.ts:73-0
Timestamp: 2025-09-24T16:32:36.609Z
Learning: In the Contentful UI Extensions SDK, both EntrySys and AssetSys interfaces extend ContentEntitySys to maintain consistency, even though assets may not typically use all inherited properties like contentType. This is an intentional design decision for API consistency.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


export type FieldType =
| 'Symbol'
| 'Text'
Expand Down