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

chore(gatsby): add Offer a base Page props type which folks can extend #21542

Merged
merged 3 commits into from
Feb 29, 2020

Conversation

orta
Copy link
Contributor

@orta orta commented Feb 18, 2020

Description

Once I started using shared fragments, all the auto-generate types tooling broke, so I went manual.

I started using this type in TypeScript v2 and figured it'd be better that I give it to Gatsby instead. Effectively gives people a base type to build on top of for describing the data in their

This is how I define my types which have both a query and custom context from createPage.

Screen Shot 2020-02-17 at 10 46 15 PM

( and yah yah, lang vs langKey - I couldn't find docs on langKey so I just used my own everywhere)

Documentation

When there's official TS docs, this should go in as a way to describe your own props - I included a bunch of examples inline which you can copy & paste later

Discussion

I do not know what these are/do:

  • "*" could be on the props, but I included it because it was on the older one (it doesn't show up in any of my props at runtime)

  • pageResources - I don't get what this does, and couldn't figure it out from some googling and reading docs - so I didn't document it and left it ambiguous but there

@orta orta requested a review from a team as a code owner February 18, 2020 04:02
@wardpeet
Copy link
Contributor

"*" could be on the props, but I included it because it was on the older one (it doesn't show up in any of my props at runtime)

We should be able to safely remove this.

pageResources - I don't get what this does, and couldn't figure it out from some googling and reading docs - so I didn't document it and left it ambiguous but there

This is an object that holds info about what the page dependencies are, component, data, ... This should have a fixed type

interface PageResources {
  component: React.Component
    json: {
      data: DataType
      pageContext: PageContextType
    },
    page: {
      componentChunkName: string,
      path: string,
      webpackCompilationHash: string,
      matchPath?: string,
    },
}

@wardpeet wardpeet added the status: awaiting author response Additional information has been requested from the author label Feb 18, 2020
@orta
Copy link
Contributor Author

orta commented Feb 18, 2020

Updated - thanks!

Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @orta!

@wardpeet wardpeet merged commit abd228e into master Feb 29, 2020
@wardpeet wardpeet deleted the dts_page_props_ branch February 29, 2020 18:17
@wardpeet wardpeet changed the title Offer a base Page props type which folks can extend chore(gatsby): add Offer a base Page props type which folks can extend Feb 29, 2020
@wardpeet
Copy link
Contributor

wardpeet commented Mar 2, 2020

Fixed in [email protected]

pieh pushed a commit that referenced this pull request Mar 10, 2020
* Update WrapPageElement*Args interface

PR #21542 introduced a new type PageProps which can also be used on WrapPageElement*Args interfaces.

* Add generics for data and context
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting author response Additional information has been requested from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants