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): Improve readability of page data / long running query warnings #37220

Merged
merged 2 commits into from
Dec 9, 2022

Conversation

LekoArts
Copy link
Contributor

@LekoArts LekoArts commented Dec 8, 2022

Description

The goal of this PR is to improve the readability of errors while HTML rendering and the warnings about long running queries. It's improved by removing verbosity that isn't that all too helpful.

  • The HTML rendering error previously output the whole page data object including the GraphQL and getServerData results. I removed these results from the error as the thrown errors by GraphQL are already good enough to find out what the error is. While yes, having the whole GraphQL result available is also a way of spotting where data is missing, normally the GraphQL errors are good enough to spot the error. There is really only a yes/no on this, as truncating the result data would be a hit or miss. Maybe we'd not truncate the relevant parts, maybe we would.
  • The long running queries warning showed the context of the query. But by already having the path to the query (either component or page) is enough to figure out the query since you can only have one of each in a file.

Related Issues

[ch59128]

LekoArts and others added 2 commits December 8, 2022 13:42
Alternative to #36876

Co-authored-by: Tyler Barnes <tylerdbarnes@gmail.com>
Co-authored-by: Daniel Lew <danielshlomolew@gmail.com>
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 8, 2022
@LekoArts LekoArts added topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Dec 8, 2022
if (!_.isEmpty(context)) {
messageParts.push(`Context: ${JSON.stringify(context, null, 4)}`)
}
if (queryJob.queryType === `page`) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The TS type said that isPage is not actually a thing on that object

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, isPage was changed to queryType when I added slice queries as then we had 3 options for queries (page, static and slice) that no longer would work with single boolean and decided type is better than adding isSlice or isStatic on top of isPage

Copy link
Contributor

Choose a reason for hiding this comment

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

So that means that since v5 we were actually not showing path or context on those long running queries I guess?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I seems like that 😆

@tyhopp tyhopp merged commit 031a08f into master Dec 9, 2022
@tyhopp tyhopp deleted the less-verbose-logs branch December 9, 2022 02:26
@LekoArts LekoArts added this to To cherry-pick in Release candidate via automation Dec 9, 2022
@marvinjude marvinjude added this to To cherry-pick in V5 Release hotfixes Dec 9, 2022
@marvinjude marvinjude removed this from To cherry-pick in V5 Release hotfixes Dec 9, 2022
marvinjude pushed a commit that referenced this pull request Dec 9, 2022
…rnings (#37220)

* show better page data information

* clean up longrunning queries

Alternative to #36876

Co-authored-by: Tyler Barnes <tylerdbarnes@gmail.com>
Co-authored-by: Daniel Lew <danielshlomolew@gmail.com>

Co-authored-by: Tyler Barnes <tylerdbarnes@gmail.com>
Co-authored-by: Daniel Lew <danielshlomolew@gmail.com>
(cherry picked from commit 031a08f)
@marvinjude marvinjude moved this from To cherry-pick to Backport PR opened in Release candidate Dec 9, 2022
marvinjude pushed a commit that referenced this pull request Dec 9, 2022
…rnings (#37220) (#37229)

* show better page data information

* clean up longrunning queries

Alternative to #36876

Co-authored-by: Tyler Barnes <tylerdbarnes@gmail.com>
Co-authored-by: Daniel Lew <danielshlomolew@gmail.com>

Co-authored-by: Tyler Barnes <tylerdbarnes@gmail.com>
Co-authored-by: Daniel Lew <danielshlomolew@gmail.com>
(cherry picked from commit 031a08f)

Co-authored-by: Lennart <lekoarts@gmail.com>
@marvinjude marvinjude moved this from Backport PR opened to Backported in Release candidate Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine)
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants