localeFallbacks not working in latest versions.

robertp-harrys commented 2 years ago

It appears that localeFallbacks don't work as expected in newer versions of gatsby-source-datocms

our defined localeFallbacks

localeFallbacks: {
  'en-DE': ['en'],
  'en-FR': ['en'],
  'en-BE': ['en'],
  'en-NL': ['en'],
  'en-GB': ['en'],
  'en-US': ['en'],
  'en-EU': ['en'],
  'fr-FR': ['fr', 'en'],
  'fr-BE': ['fr', 'en'],
  'nl-NL': ['nl', 'en'],
  'nl-BE': ['nl', 'en'],
  'de-DE': ['de', 'en'],
}

Below Im posting 3 examples of data being returned differently for different versions of those plugins. The query is the same across all 3 examples.

Example 1 (Working as expected):

gatsby-source-datocms@3.0.15 datocms-client@3.5.9

Screenshot from /___graphql:

Example 2 (Not Working): gatsby-source-datocms@4.0.4 datocms-client@3.5.9

Screenshot from /___graphql:

Example 3 (latest versions - Not Working) gatsby-source-datocms@4.0.4 datocms-client@3.5.21

Screenshot from /___graphql:

michaellopez commented 2 years ago

We hit this problem too. We're interested in contributing a PR if maintainers can point us in the right direction. @stefanoverna @matjack1

michaellopez commented 2 years ago

I think this comes down to these lines https://github.com/datocms/gatsby-source-datocms/blob/master/src/hooks/sourceNodes/createNodeFromEntity/item/index.js#L22-L26 from https://github.com/datocms/gatsby-source-datocms/commit/76520c4671c5fb7bc4420313f62f2839a44e9c8a

@robertp-harrys For us this change is indeed correct. It is meant to create less nodes (faster Gastby sourcing) and more properly reflect the data models defined in Dato. In our case we had a singleton model which did not have any localised fields enable. As you show with version of this plugin before https://github.com/datocms/gatsby-source-datocms/commit/76520c4671c5fb7bc4420313f62f2839a44e9c8a it created Gatsby Nodes for all locales defined in Dato regardless of whether or not the model defined any localised fields. After https://github.com/datocms/gatsby-source-datocms/commit/76520c4671c5fb7bc4420313f62f2839a44e9c8a it now does the correct thing, no localised fields in the model? Then no Gatsby Node for those locales, resulting in fewer nodes and better performance.

In our case our code relied on these "extra nodes" which was incorrect, we should rely on this data from other sources.

Your case looks like something similar to ours. From my understanding of the code in this plugin you should not expect the plugin to create Gatsby Nodes for all your locales that are a copy of your fallback locales, but rather that the locales that do not have the translations get the VALUES from the first available fallback locale. So if you query for the fr-BE locale for an item and that item does not have content input for that locale in Dato, the VALUES in the response will be those from either the fr locale, or the en locale if fr is missing content. If en does not have content either the value will be the empty string. But none of this logic will execute unless the field in question has localisation enabled, otherwise it will just use the main locale (the first one in the list in settings in Dato). With this reasoning it might help a bit of a workaround to define your fallback locale as the first locale so it is the main locale, maybe that can help a bit (it won't solve your underlying issue though).

Maybe some of this reasoning might help you understand your underlying issue. At this moment I think it is a data issue or configuration issue on your end or some misunderstanding of how the localisation feature works at Dato.

It would be nice if any official dato member could confirm my findings above.

stefanoverna commented 2 years ago

That is correct @michaellopez.

We're currently talking with Gatsby to see if we can do any better than this.

Ideally, we'd like to replicate the way localization works with our GraphQL API. This would allow us to generate only one Gatsby node per record, and let the developer customize the locale (and fallback locales) to be returned on a per-field level... but some pieces are missing in the Gatsby API atm.

I'll keep you posted!

jpriceharrys commented 2 years ago

@michaellopez @stefanoverna Thank you for your input, however the explanation for how it should work is the same as our assumption today - and it makes total sense to only create nodes when the model has localisable fields.

However, we have a model with a localisable field (called "count"), but the fallback content only returns if we actually add the locale to the record via the CMS - so given the locale mapping @robertp-harrys gave above; if we just want the fr-FR content to be the same as the en locale we have to add an entire empty record in the CMS and leave the "count" field blank (which doesn't work when the field is required because you can't save an empty record).

For example with the following setup in the CMS (note the count is localisable, and I haven't added the 'fr-FR' locale so the expected final fallback is 3):

and this fallback config:

localeFallbacks: {
  ... other locales ...
  'fr-FR': ['fr', 'en'],
}

Running this gql code from graphiql:

datoCmsMyModel(locale: {eq: "fr-FR"}) {
  count
}

with v4.0.4 the value of count is null, but on v3.0.15 the value is coming back as expected (in my example 3).

I'm not sure if that makes sense so please let me know if not. If the solution is that we have to add every locale to any records then it seems like fallbacks are a little redundant and could result in a poor content editor experience / with lots of duplication.

michaellopez commented 2 years ago

@jpriceharrys If you debug the lines I linked above the logic says the following:

If there is no localised field, use the first locale defined in Dato
If there IS a localised field, check if the model requires all locales to have a value
If all locales are required
- Generate a node for all locales because the source plugin can expect data to be defined for all locales
If all locales are NOT required
- Check the entity for all locales that have a value and generate a node for each of those (I haven't debugged this with data, but this is what I think happens for the line Object.keys(entity[camelize(firstLocalizedField.apiKey)]);, @jpriceharrys please verify if you can)

If my understanding is correct, given your more detailed post above, I now consider this a bug with gatsby-source-datocms. Because just as you say, you are required to fill in a value for a locale for a Gatsby Node to be created for that locale.

The localeFallbacks option will only get processed for locales that have a value, which kind of defeats the purpose. @stefanoverna do you agree?

If so should the logic change to include localeFallbacks in the decision for localesToGenerate? I think that all locales that have a fallback defined in localeFallbacks should get a Gatsby Node generated with the fallback value. I can probably get a PR drafted if this is something the maintainers are willing to merge.

stefanoverna commented 2 years ago

Yes, it is a bug. Happy to review a MR on this, thanks!

— Stefano Verna Founder and CEO @ DatoCMS

--- original message --- On June 8, 2022 at 4:01 PM GMT+2 @.*** wrote:

@jpriceharrys If you debug the lines I linked above the logic says the following:

If there is no localised field, use the first locale defined in Dato

If there IS a localised field, check if the model requires all locales to have a value

If all locales are required Generate a node for all locales because the source plugin can expect data to be defined for all locales

If all locales are NOT required Check the entity for all locales that have a value and generate a node for each of those (I haven't debugged this with data, but this is what I think happens for the line Object.keys(entity[camelize(firstLocalizedField.apiKey)]);, @jpriceharrys please verify if you can)

If my understanding is correct, given your more detailed post above, I now consider this a bug with gatsby-source-datocms. Because just as you say, you are required to fill in a value for a locale for a Gatsby Node to be created for that locale.

The localeFallbacks option will only get processed for locales that have a value, which kind of defeats the purpose. @stefanoverna do you agree?

If so should the logic change to include localeFallbacks in the decision for localesToGenerate? I think that all locales that have a fallback defined in localeFallbacks should get a Gatsby Node generated with the fallback value. I can probably get a PR drafted if this is something the maintainers are willing to merge.

—

Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you were mentioned.Message ID: @.***> --- end of original message ---

stefanoverna commented 2 years ago

Alright, we just shipped v5.0.0-0, which:

generates only one node per record (yay!);
introduces a new way to fetch localized content through locale and fallbackLocales arguments. This aligns the the plugin with the way localization works in our Content Delivery API, and should solve the flexibility issues encountered so far.

You can read all the details here: https://github.com/datocms/gatsby-source-datocms/tree/experiment#localized-fields

It would really help if you could provide a feedback! 🥳

stefanoverna commented 1 year ago

Hey @michaellopez @jpriceharrys any news? :)

jpriceharrys commented 1 year ago

Apologies @stefanoverna, this slipped off my radar. I will test it now and see how it goes, thank you!

jpriceharrys commented 1 year ago

@stefanoverna just to check that I am getting this right - I have updated to version 5, but do we now need to change all queries we make to Dato to include the fallbackLocales array? It doesn't appear to be pulled from the gatsby-config.js file anymore (I tried renaming it to fallbackLocales as well as localeFallbacks because I noticed the commit renamed it, but it still doesn't work until I add it manually.

So for example the above query has to change to:

fallback: datoCmsMyModel(locale: "fr-FR", fallbackLocales: [“fr”, “en"]) {
  count
}

Note: I did realise that after I add the fallbackLocales once, I can remove it (the result must be cached), but when restarting the server again it returns null until I add it back.

michaellopez commented 1 year ago

Hey @michaellopez @jpriceharrys any news? :)

Hi @stefanoverna, I assume your release of v5.0.0-0 fixed this, so I did not look further into a PR addressing the issues I outlined. Are you still interested in this PR?

I'll leave testing v5 for the outlined issues to @jpriceharrys and his colleagues.

stefanoverna commented 1 year ago

@jpriceharrys that is correct. Is this new strategy solving your previous querying issues? Are you able to retrieve everything you need now?

jpriceharrys commented 1 year ago

Hi @stefanoverna. This looks like the fallbacks are working, however i'm not sure it's a path we would be able to move forward with due to now being a lot more cumbersome having to add the fallback locales to every query (we currently have ~60 queries throughout our codebase) plus we will have to make it so that our fallbacks are changeable based on the original requested locale (because the fallback for fr-FR will be different for the fallbacks for de-DE).

However, my biggest concern is when it comes to adding new languages and adding more fallbacks. In the past it was very simple as we just had to update the gatsby-config.js file but now we would have to change every query again.

Is there any way to keep the setup as before in the gatsby-config? I don't mind having to change our queries structure from locale: { eq: $locale } to simply locale: $locale.

This might not be related to your code change (so please let me know if not - just wanted to highlight) when running the following query:

allDatoCmsMyModel(filter: {locale: {eq: "en"}}, sort: {fields: key}) {
  nodes {
    key
  }
}

My development server crashes with the follow error (removing the sort works OK):

success Writing page-data.json files to public directory - 0.201s - 1/1 4.97/s

 ERROR

Cannot read property 'localeState' of undefined

  TypeError: Cannot read property 'localeState' of undefined

  - index.js:42 getI18n
    [client]/[gatsby-source-datocms]/hooks/sourceNodes/createTypes/item/index.js:42:38

  - index.js:126 resolver
    [client]/[gatsby-source-datocms]/hooks/sourceNodes/createTypes/item/index.js:126:24

  - resolvers.ts:683 Object.resolve
    [client]/[gatsby]/src/schema/resolvers.ts:683:20

  - node-model.js:891 resolveField
    [client]/[gatsby]/src/schema/node-model.js:891:19

  - node-model.js:812 resolveRecursive
    [client]/[gatsby]/src/schema/node-model.js:812:28

  - node-model.js:484 LocalNodeModel._doResolvePrepareNodesQueue
    [client]/[gatsby]/src/schema/node-model.js:484:38

  - node-model.js:443
    [client]/[gatsby]/src/schema/node-model.js:443:22

  - task_queues.js:77 processTicksAndRejections
    internal/process/task_queues.js:77:11

stefanoverna commented 1 year ago

@jpriceharrys thanks for the feedback!

First of all, regarding the error: would you be able to share your Gatsby project, so that I can take a better look at it?

I'm not sure we can return to the settings in gatsby-config.js... and we actually like to be 1:1 with the way our own GraphQL API works. Should it be pretty straght-forward to "extract" the fallback logic so that you can reuse it whenever you need to fetch localized content? Again, if I can take a look at the code maybe I can see the issue better and/or provide a solution.

jpriceharrys commented 1 year ago

Hi @stefanoverna. I'll try to get a small POC with the sort error above and if I can reproduce it there I'll share with you.

jpriceharrys commented 1 year ago

@stefanoverna Please find a very simple dummy app here to show the issue. The steps to reproduce are in the README but if there is anything else, please let me know. We're using a test dato instance API read-only key which you can use too so you can see the structure of the model.

stefanoverna commented 1 year ago

Submitted to Gatsby team.. really appreciated @jpriceharrys... hope they can help us understand the cause :/

pieh commented 1 year ago

For the crashing part - we found culprit in gatsby using your repro (thank you!) - you can check https://github.com/gatsbyjs/gatsby/pull/36552 to follow fixes being made to gatsby. With that I was at least able to execute query described in README:

stefanoverna commented 1 year ago

awesome!

pieh commented 1 year ago

Above fix was just published in gatsby@next (concrete version right now is gatsby@4.24.0-next.0).

stefanoverna commented 1 year ago

The new v5 is out! Closing this one!

datocms / gatsby-source-datocms

localeFallbacks not working in latest versions. #196