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

docs: Remove note about stringified options #13440

Merged
merged 1 commit into from
Apr 18, 2019
Merged

Conversation

CanRau
Copy link
Contributor

@CanRau CanRau commented Apr 18, 2019

I'm not entirely sure about this, I'm aware of the origin #3987 yet this PR I made proofs the opposite #12060
Only thing I can think of right now that browser APIs get stringified, couldn't verify this so far, this #3834 is suggesting they get stringified and it's the reason the note was added to the docs in the first place..

Couldn't just leave my confusion alone and thought doing it in a PR might be useful if it's actually not valid anymore so it can be merged directly, otherwise feel free to reject it of course 👍

Description

Related Issues

I'm not entirely sure about this, I'm aware of the origin #3987 yet this PR I made proofs the opposite #12060 
Only thing I can think of right now that browser APIs get stringified, couldn't verify this so far, this #3834 is suggesting they get stringified and it's the reason the note was added to the docs in the first place..

Couldn't just leave my confusion alone and thought doing it in a PR might be useful if it's actually not valid anymore so it can be merged directly, otherwise feel free to reject it of course 👍
@CanRau CanRau requested a review from a team April 18, 2019 08:45
@pieh
Copy link
Contributor

pieh commented Apr 18, 2019

Browser APIs options get JSON.stringified (

options: ${JSON.stringify(plugin.options)},
)
and SSR APIs options get JSON.stringified (
options: ${JSON.stringify(plugin.options)},
)

Node APIs are not getting stringified because they executed in the same context, so they are just used from memory as they are

@CanRau
Copy link
Contributor Author

CanRau commented Apr 18, 2019

Okay, good to know, thanks for clarifying 🙏
Then the note either stays or could get an update like

Note that plugin options of gatsby-ssr.js and gatsby-browser.js get stringified by Gatsby, so they cannot be functions.

But maybe it's not the right place 🤔 I think it's a little bit misleading as they can be in certain cases..
What do you think?

Copy link
Contributor

@sidharthachatterjee sidharthachatterjee left a comment

Choose a reason for hiding this comment

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

@CanRau While like @pieh correctly mentioned, options do indeed get stringified in case of gatsby-ssr and gatsby-browser, in the context of this doc, it's okay to remove the note since

  • It's not always true
  • It's not relevant to the user reading this

Merging this in! Thank you 👍

@sidharthachatterjee sidharthachatterjee merged commit 8f92062 into master Apr 18, 2019
@sidharthachatterjee sidharthachatterjee deleted the CanRau-patch-1 branch April 18, 2019 18:08
@mike1808
Copy link

I was trying to pass function for gatsby-browser and I faced this issue. I think, it's worth mentioning in the docs that options for ssr and browser are stringified and you cannot pass functions.

@abumalick
Copy link
Contributor

I think it is worth mentioning somewhere in the plugin docs too.

I don't know in which section it would fix through

https://www.gatsbyjs.org/docs/creating-plugins/

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.

5 participants