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): Content Sync DSG bug #34799

Merged
merged 2 commits into from
Feb 14, 2022
Merged

fix(gatsby): Content Sync DSG bug #34799

merged 2 commits into from
Feb 14, 2022

Conversation

TylerBarnes
Copy link
Contributor

I recently discovered Content Sync no longer works with DSG. I could've sworn it used to but it doesn't now. This fix will be less performant for large sites. I'm going to follow this PR up with another that will improve performance but since we have a marketing release of Content Sync next week I don't want to make too many code changes before then. This is the most minimal change required to make Content Sync work w/ DSG.

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Feb 11, 2022
@TylerBarnes TylerBarnes removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Feb 11, 2022
@TylerBarnes TylerBarnes requested a review from veryspry February 11, 2022 23:26
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

DSG has the same issue as query-on-demand where queries are not ran so it's going to be difficult to make this work unless people give you ownerId or use fs routes

@TylerBarnes
Copy link
Contributor Author

We're on the same page about that @wardpeet :) I think the perf improvement to be made here is to iterate over all pages only 1 time before we start processing node manifests and make a Map or two to quickly look up the needed info. In this change we're now iterating on all pages for each node manifest which isn't great.

@wardpeet wardpeet merged commit bfd04d3 into master Feb 14, 2022
@wardpeet wardpeet deleted the fix/content-sync-dsg branch February 14, 2022 20:06
imjoshin pushed a commit that referenced this pull request Feb 15, 2022
imjoshin pushed a commit that referenced this pull request Feb 15, 2022
(cherry picked from commit bfd04d3)

Co-authored-by: Tyler Barnes <tylerdbarnes@gmail.com>
@imjoshin
Copy link
Contributor

Published in gatsby@4.7.2

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.

4 participants