Skip to content

Conversation

@sbardian
Copy link
Contributor

Description

Was using wrapRootElement api and had need of pluginOptions in my gatsby-plugin-breadcrumb plugin. Documentation didn't show pluginOptions as a parameter, but reviewing the code it seems all Browser and SSR api's do get pluginOptions sent as a parameter, from what I can see. I was able to use them successfully with wrapRootElement so thought adding it to the docs would be helpful.

Related Issues

N/A

@sbardian sbardian requested a review from a team March 13, 2019 17:34
@@ -1,5 +1,6 @@
/**
* Called when the Gatsby browser runtime first starts.
* @param {Object} pluginOptions
Copy link
Contributor

Choose a reason for hiding this comment

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

This is weird, but pluginOptions I think will be second paramater, and first one will actually be undefined. And this is the same for all APis that doesn't really pass any args (so to onInitialClientRender, registerServiceWorker, disableCorePrefetching, replaceHydrateFunction as well)

Copy link
Contributor Author

@sbardian sbardian Mar 14, 2019

Choose a reason for hiding this comment

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

Agreed a little weird, but correct. I pulled down your update and added

@param undefined _

for the first parameter on those you listed (not sure that is the best way to display that though), and including the @param {Object} pluginOptions after. I then tried gatsby develop just to be sure everything was good, and I'm getting the error:

GraphQL Error Fragment "DocumentationParamsFieldsFragment" cannot be spread here as objects of type "params_2" can never be of type "DocumentationJs".

  file: /Users/sbardian/stuff/dev/javascript/PRs/gatsby/www/src/components/api-reference/params.js

  35 |           static {
  36 |             ...DocumentationParamBase
  37 |           }
  38 |         }
  39 |       }
  40 |     }
  41 |   }
  42 |   fragment DocumentationParamsFragmentR1 on DocumentationJs {
  43 |     ...DocumentationParamsFieldsFragment
  44 |     params {
> 45 |       ...DocumentationParamsFieldsFragment
     |       ^
  46 |     }
  47 |     properties {
  48 |       ...DocumentationParamsFieldsFragment
  49 |     }
  50 |     members {
  51 |       static {
  52 |         ...DocumentationParamsFieldsFragment
  53 |       }
  54 |     }
  55 |     type {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to be something upstream. Pushed and will rebase after it's fixed.

@sbardian
Copy link
Contributor Author

sbardian commented Apr 5, 2019

@pieh bump to avoid stale bot. Let me know if you would like updates/etc. No rush.

@pieh pieh self-assigned this Apr 13, 2019
@sbardian sbardian requested a review from a team April 13, 2019 13:08
@pieh
Copy link
Contributor

pieh commented Apr 13, 2019

Hey @sbardian!

Sorry for getting quiet on this one.

I went ahead and apply few changes:

  • I added API function/callback signature
  • Added more context to pluginOptions argument and "no arg" cases
  • Also updated/adjusted few jsdocs while I was at it:

Example of "no arg" case:
Screenshot 2019-04-13 at 15 04 49

Example of "full arg" case:
Screenshot 2019-04-13 at 15 05 21

@pieh
Copy link
Contributor

pieh commented Apr 13, 2019

I also unified object/Object (I have no preference which one to use - just so it's consistent)

@sbardian
Copy link
Contributor Author

@pieh not a problem. Looks nice and clean to me! Let me know if you would like me to jump in on anything, but looking good 👍

Copy link
Contributor Author

@sbardian sbardian left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@pieh pieh changed the title Update browser and ssr api docs to include pluginOptions parameter docs: Update browser and ssr api docs to include pluginOptions parameter Apr 13, 2019
@pieh pieh merged commit 4d763dc into gatsbyjs:master Apr 13, 2019
@sbardian sbardian deleted the docs/browser-ssr-api branch April 13, 2019 15:51
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