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

perf(gatsby): Minify page-data #35578

Merged
merged 2 commits into from May 5, 2022
Merged

perf(gatsby): Minify page-data #35578

merged 2 commits into from May 5, 2022

Conversation

me4502
Copy link
Contributor

@me4502 me4502 commented May 3, 2022

Description

This minifies the page-data.json files, rather than including whitespaces and newlines. This is a very minor change regarding bytes of the file, but there's really no reason not to do this. The file was also already partially minified, as the final closing brace would end up at the end of the preceding line. Other JSON files outputted by Gatsby are all minified, so this brings page-data in line with the rest.

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label May 3, 2022
@LekoArts LekoArts added topic: performance Related to runtime & build performance and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels May 3, 2022
@KyleAMathews
Copy link
Contributor

We left it unminified in the past as it's nice to be able to easily read the files on disk + with gzip it doesn't make much difference afaik. What is the size difference of the file over the network w/ gzip?

@me4502
Copy link
Contributor Author

me4502 commented May 4, 2022

Given the useful data in it is already minified (the query result), and other files (static queries, etc) are too, and also most people who will be looking at it would be using it through a tool that supports beautifying the file, I'm unsure whether it makes sense to do it for that reason.

Regarding stats, on an example file it saved 14 bytes gzipped (and around 4x that in raw file size). It's not a massive saving, but for a site with 20k pages that's over 1mb saved on disk. If a page links to ~20 other pages (such as a blog index or similar), that then becomes 292 bytes saved. For 50k page loads, that's then 15mb. It's a minor saving but it somewhat feels like it doesn't have downsides, so it's free bytes.

If you'd rather keep it as-is without minifying it that's fine, I only made the PR because I noticed the file wasn't minified in my network tab and made a PR to change that as I didn't personally see a downside. If it's wanted behaviour that's a different matter.

@LekoArts
Copy link
Contributor

LekoArts commented May 4, 2022

Given the useful data in it is already minified (the query result), and other files (static queries, etc) are too, and also most people who will be looking at it would be using it through a tool that supports beautifying the file, I'm unsure whether it makes sense to do it for that reason.

I think these are good arguments in favor of doing that change 👍

@KyleAMathews
Copy link
Contributor

Yeah I think I left it unminified early on with Gatsby v1 as I was looking at the generated files a lot to debug stuff. But these days, query running is a very mature sub-system so it's not common to look directly at the page-data.json files.

@LekoArts LekoArts changed the title perf: minify page-data perf(gatsby): Minify page-data May 5, 2022
@LekoArts LekoArts merged commit 8bad9b3 into gatsbyjs:master May 5, 2022
@me4502 me4502 deleted the chore/minify-page-data branch May 5, 2022 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: performance Related to runtime & build performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants