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

feat(gatsby-source-contentful): add locale filter #12939

Merged
merged 10 commits into from
Apr 16, 2019

Conversation

stoltzrobin
Copy link
Contributor

Description

This allows the user to filter out locales that are not needed for the build. For example, if the space in Contentful is rather large with very many locales the javascript memory easiely runs out of memory.

Related Issues

#12938

@stoltzrobin stoltzrobin changed the title Topics/add locale filter [gatsby-source-contentful] Add locale filter to contenful Mar 29, 2019
@stoltzrobin stoltzrobin requested a review from thibautRe March 29, 2019 10:37
Copy link
Contributor

@Khaledgarbaya Khaledgarbaya left a comment

Choose a reason for hiding this comment

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

The idea make sense LGTM

@pieh
Copy link
Contributor

pieh commented Mar 29, 2019

Is it possible to instruct contentful client to fetch data just for given locale? If not this seems good to go.

@pieh
Copy link
Contributor

pieh commented Mar 29, 2019

I think I just answered my own question when checking https://www.contentful.com/developers/docs/concepts/sync/

Because the Sync API retrieves all localized content, it might be better to use the delivery API to retrieve results of a single locale.

@pieh pieh changed the title [gatsby-source-contentful] Add locale filter to contenful feat(gatsby-source-contentful: add locale filter Mar 29, 2019
@pieh pieh changed the title feat(gatsby-source-contentful: add locale filter feat(gatsby-source-contentful): add locale filter Mar 29, 2019
@stoltzrobin
Copy link
Contributor Author

@pieh that was my initial though as well but as shown in the link that you posted it's not really the idea of the .sync()function. Maybe in the future it should be nice to support to pass queries in the sync call as well

@pieh
Copy link
Contributor

pieh commented Mar 29, 2019

Looking good now, gotta check it out in practice before merging

@stoltzrobin
Copy link
Contributor Author

@pieh nice 👍

@thibautRe
Copy link
Contributor

Hello @pieh, just checking if there's anything I can do to help get this approved and merged :)

@pieh pieh self-assigned this Apr 16, 2019
@pieh
Copy link
Contributor

pieh commented Apr 16, 2019

Thanks @stoltzrobin, @Khaledgarbaya and @thibautRe for help with getting it merge-ready!

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