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(gatsby): Update types for WrapPageElement*Args #22120

Merged
merged 2 commits into from
Mar 10, 2020
Merged

fix(gatsby): Update types for WrapPageElement*Args #22120

merged 2 commits into from
Mar 10, 2020

Conversation

adambrgmn
Copy link
Contributor

Description

Hi, and thanks for a great project! This (my first) PR is just a small thing proposing an update to the typescript types. It should possibly be opened as an issue first, but since the changes are so small I thought it would be easiest to make it a PR directly – a PR which of course can be closed if I've misunderstood anything.

I propose updated types for the interfaces WrapPageElementBrowserArgs and WrapPageElementNodeArgs with a more detailed props definition. PR #21542 introduced a new detailed interface for page props and I figured this interface can also be used for the types for wrapPageElement.

I could of course be wrong (the docs and the current interface only specify props as a vague object) but my guess is that props in wrapPageElement is the same props passed to pages. Please correct me if I'm wrong.

Documentation

The arguments passed to wrapPageElement are documented in two places – browser and ssr. Would you like me to touch on these as well?

Related Issues

No related issues found.

PR #21542 introduced a new type PageProps which can also be used on WrapPageElement*Args interfaces.
@adambrgmn adambrgmn requested a review from a team as a code owner March 9, 2020 23:03
@adambrgmn adambrgmn changed the title Adambrgmn update wrap types Update types for WrapPageElement*Args Mar 9, 2020
@pieh pieh changed the title Update types for WrapPageElement*Args fix(gatsby): Update types for WrapPageElement*Args Mar 10, 2020
@pieh pieh self-assigned this Mar 10, 2020
Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

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

This is mostly correct, but there are few differences between SSR and Browser - mainly in SSR we don't have pageResources object that is there in browser (but this is not in scope of this PR).

Let's get this in.

I feel like there would need to be some restructure to GatsbyBrowser interface, because those generic templates will need to be used manually for now, but we can work on that incrementally.

@pieh pieh merged commit 97fa23e into gatsbyjs:master Mar 10, 2020
@gatsbot
Copy link

gatsbot bot commented Mar 10, 2020

Holy buckets, @adambrgmn — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

  1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. We’ve got Gatsby t-shirts, stickers, hats, scrunchies, and much more. (You can also unlock even more free swag with 5 contributions — wink wink nudge nudge.) See gatsby.dev/swag for details.
  2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. Accept the invite by visiting https://github.com/orgs/gatsbyjs/invitation. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!

@adambrgmn
Copy link
Contributor Author

@pieh thanks! I might take a stab at it since I share your "concerns"!

How often does a release happen (read; when will this be published)? 😬

@pieh
Copy link
Contributor

pieh commented Mar 10, 2020

We release couple times a day, I will release soon and let you know ;)

@pieh
Copy link
Contributor

pieh commented Mar 10, 2020

Published [email protected]

@adambrgmn adambrgmn deleted the adambrgmn-update-wrap-types branch March 16, 2020 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants