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

Theme.getUrls type annotation is misleading #2318

Closed
nex3 opened this issue Jun 27, 2023 · 2 comments
Closed

Theme.getUrls type annotation is misleading #2318

nex3 opened this issue Jun 27, 2023 · 2 comments
Labels
bug Functionality does not match expectation

Comments

@nex3
Copy link

nex3 commented Jun 27, 2023

The type annotation for Theme.getUrls() is:

abstract getUrls(project: ProjectReflection): UrlMapping[];

and UrlMapping is:

export class UrlMapping<Model = any> {

This implies that a theme can return a UrlContext with any model value. However, this condition:

if (page.model instanceof Reflection) {
page.contents = this.theme!.render(page, template);
} else {
throw new Error("Should be unreachable");
}

throws an error if the model isn't specifically a Reflection. It seems like either getUrls() should specifically return a UrlMapping<Reflection>, or Theme should be made generic over the model type.

(My use-case here is adding a generated file to my theme that isn't based on a model at all, but rather on some side-loaded data, so I'd prefer the latter, but I can work around the former.)

@nex3 nex3 added the bug Functionality does not match expectation label Jun 27, 2023
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jun 30, 2023

Eventually, I'd like to make the current return type of getUrls truly valid, IIRC it currently uses any due to some variance issues that make UrlMapping<Reflection> non-assignable to UrlMapping<DeclarationReflection>... I've been having too much fun at work lately to want to work much on code stuff on the weekend.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jul 29, 2023

Work on #247 will eventually enable that ^ for now, fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Functionality does not match expectation
Projects
None yet
Development

No branches or pull requests

2 participants