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

fix(astro): strongly type Astro.self #7866

Merged
merged 5 commits into from
Aug 14, 2023
Merged

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Jul 29, 2023

Changes

Basically trying to strongly type Astro.self so we can get completions in the language tools.

This is the safest non-breaking way I could think of doing it, since AstroComponentFactory should work as it did before if you don't specify any type parameters.

I'm not entirely sure how AstroComponentFactory ends up working in TSX (in the type system) since it doesn't look like the shape of a JSX element. so this may also be the completely wrong solution. either way, if someone could explain that gap in my knowledge, that'd be super helpful

This should fix withastro/language-tools#612

Testing

Couldn't find any existing tests for the types package. If there are some, i'm happy to add to them.

Docs

N/A since the behaviour should remain the same

@43081j 43081j requested a review from a team as a code owner July 29, 2023 21:54
@changeset-bot
Copy link

changeset-bot bot commented Jul 29, 2023

🦋 Changeset detected

Latest commit: eb216b1

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Jul 29, 2023
@ematipico
Copy link
Member

cc @Princesseuh

@Princesseuh
Copy link
Member

So, in that type there's a bit of a confusion where sure, AstroComponentFactory is the proper type in Astro itself, but it's not the proper type in the editor tooling (for multiple reasons).

Ultimately, the type of Astro.self only matters for editor tooling, because it's 100% handled in the compiler anyway. So what I would suggest is that instead, is to add a type param here:

export interface AstroGlobal<Props extends Record<string, any> = Record<string, any>>
that types self. Then in the compiler, in the TSX we'll pass a typeof of the default function to the type param.

@43081j
Copy link
Contributor Author

43081j commented Jul 31, 2023

have updated this to take a Self which defaults to the component factory it used before to keep it backwards compatible.

the associated compiler change is here: withastro/compiler#843

@Princesseuh Princesseuh merged commit d1f7143 into withastro:main Aug 14, 2023
15 of 17 checks passed
@astrobot-houston astrobot-houston mentioned this pull request Aug 14, 2023
@43081j 43081j deleted the self-types branch August 14, 2023 14:27
SudoCat pushed a commit to SudoCat/astro that referenced this pull request Aug 14, 2023
Co-authored-by: Erika <3019731+Princesseuh@users.noreply.github.com>
Co-authored-by: Princesseuh <princssdev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Astro.self props type safety
4 participants