fix see more

[mruwnik]

No description provided.

[jrhender]

A few suggestions

[jrhender]

I believe it would be more type-safe down the line to use unknown here. This change doesn't seem to break anything in the existing code and so should be easy to apply now.

Suggested change

	const head = (item: any | any[]) => {
	const head = (item: unknown | unknown[]) => {

[jrhender]

This type change doesn't seem necessary as far as I can tell. The new extractIcon function returns string | undefined, so I think that is a suitable type here.

Suggested change

	Icon?: Entity | string | Entity[]
	Icon?: string

[mruwnik]

this is here mainly to document what Coda returns. Which can be either the raw image (base64 encoded), an url to the image, a reference to a Coda item or a list of references to Coda items... I'm not sure what exactly causes what

[jrhender]

I think it would be nice to avoid the val as casts to benefit from type checking of val in the future. I've suggested an isStringOrStringArray type guard to do this, though there might be a more elegant way.

Suggested change

	const item = head(val)
	if (typeof item === 'string') return extractText(val as string)
	if (item) return (item as Entity).url
	return undefined
	}
	if (isStringOrStringArray(val)) return extractText(val)
	const item = head(val)
	if (item) return item.url
	}
	const isStringOrStringArray = (x: unknown): x is string | string[] => {
	const item = head(x)
	return typeof item === 'string'
	}

I also think the last return undefined isn't necessary. I think undefined will be returned if none of the if cases are matched, even without this line.

[mruwnik]

the thing is that I know that if it's not a string and not undefined, then it's going to be an Entity at this point. Typescript doesn't know that.

Wouldn't that be void?

[jrhender]

the thing is that I know that if it's not a string and not undefined, then it's going to be an Entity at this point. Typescript doesn't know that.

Ah maybe part of the confusion here is that in my local code where I was trying this out, I also changed the signature of the head function to const head = <T,>(item: T | T[]): T. With this, I'm pretty sure that typescript does know that item is an Entity at that point. You can see in screenshot below that my local compiler is showing the type of item as that point as Entity.

The way I'm reasoning about it is that the parameter type is undefined | string | string[] | Entity | Entity[] and then:

1. if (!val) return val removes the possibility of undefined
2. if (isStringOrStringArray(val)) return extractText(val) removes the possibility of string | string[]
3. head (with the signature change above) narrows Entity | Entity[] to Entity

At any rate, I'll make a PR at some point to improve the head function so that it's easy to make future code more type safe 😊 . Probably less important getting the UI redesign out.

Wouldn't that be void?

No, I don't think so. I think void is a typescript type but the actual return type is undefined. I wrote this JS fiddle as a quick test: https://jsfiddle.net/#&togetherjs=M9fHnDlskK

[mruwnik]

ohhh, I like that version of head!