chapter-three / next-drupal

Next.js for Drupal has everything you need to build a next-generation front-end for your Drupal site: SSG, SSR, and ISR, Multi-site, Authentication, Webforms, Search API, I18n and Preview mode (works with JSON:API and GraphQL).
https://next-drupal.org
MIT License
606 stars 172 forks source link

defaultLocale missing language prefix #286

Open pinoniq opened 1 year ago

pinoniq commented 1 year ago

The defaultLocale of next.js is not always the same as the Drupal one. So removing the language prefix when retreiving paths will give a 404.

What is the reason for removing the prefix from the slug for defaultLocale? https://github.com/chapter-three/next-drupal/blob/main/packages/next-drupal/src/utils.ts#L117-L120

Drupal can also add the prefix for the defaultLocale, so shouldn't this be a configuration?

shadcn commented 1 year ago

I see. We assume that the defaultLocale on the Next.js site is always the same as the Drupal one.

The defaultLocale of next.js is not always the same as the Drupal one.

Hmm, do you mean your Next.js site is pulling non-default content and using it for for defaultLocale?

Untitled-2022-03-23-1048

sinyayadynya commented 1 year ago

@shadcn I think that what @pinoniq try to explain is that on Drupal URL language detection configuration (/admin/config/regional/language/detection/url) the default language CAN have a langcode prefix too:

Screen Shot 2022-11-09 at 12 22 09

And in that case, then all default language nodes return a 404 error page with Next Drupal.

What @pinoniq suggest is to make that langcode prefix configurable in Next Drupal. Maybe there is already a way to do that, but it is not obvious and I could not find a fix neither.

Any help on that issue would be welcome, as I can't really remove the English (en) path prefix (Default language) on existing project.

shadcn commented 1 year ago

@sinyayadynya Any chance you can help me reproduce this? What should I configure on the Drupal site?

sinyayadynya commented 1 year ago

@shadcn sure I would be glad if I can help. Here is what is needed on the Drupal side config:

Then with some content in default English language, you should be able to reproduce the 404 error.

Screen Shot 2022-11-09 at 14 40 17

After trying the marketing and umami examples and watching the Bay Area Drupal Camp demo, I came back to the basic-starter app and noticed that if I used it with my Umami Drupal demo (without 'en' prefix for the default language) it was working as expected. But as soon as I'm connecting it to another project with 'en' prefix for the default language, then it fails.

@pinoniq probably pointed the right piece of code where that happen. Now I'm too fresh with NextJS to say if there is a simple way to fix that either in config or in components.

sinyayadynya commented 1 year ago

I have tried also to add this to the next config:

module.exports = {
    nextConfig,
    i18n: {
        locales: ['default', 'en', 'es', 'fr'],
        defaultLocale: 'default',
        localeDetection: false,
    },
    trailingSlash: true,
};

But then I will get this error, with double /en/en/ :

Server Error
Error: Failed to fetch JSON:API index at http://dev.site.io/en/en/jsonapi - Unexpected token < in JSON at position 0
shadcn commented 1 year ago

What if you use the /en/jsonapi as apiPrefix? Could this work as a temp solution?

export const drupal = new DrupalClient(process.env.NEXT_PUBLIC_DRUPAL_BASE_URL, {
  apiPrefix: "/en/jsonapi",
})

https://next-drupal.org/docs/configuration#apiprefix

sinyayadynya commented 1 year ago

Unfortunately also getting:

Error: Failed to fetch JSON:API index at http://dev.drupalsite.io/en/en/jsonapi - Unexpected token < in JSON at position 0

with double /en/en/ in the path, when adding apiPrefix: "/en/jsonapi", into drupal.ts

(while reverting the previous next config change mentioned in https://github.com/chapter-three/next-drupal/issues/286#issuecomment-1308611254)

It is either too many or too few prefix.

martdavidson commented 1 year ago

Just came across this as webforms requires Interface text language detection of url to serve the translated form: https://www.drupal.org/project/webform/issues/3175374

Enabling that of course raises this issue where next-drupal is trying to fetch ${NEXT_PUBLIC_DRUPAL_BASE_URL}/jsonapi instead of ${NEXT_PUBLIC_DRUPAL_BASE_URL}/en-ca/jsonapi for the default locale (en-ca is our default). The former 404s.

Anyway, I think my issue is exactly this ticket, fingers crossed on the pull request you opened. Thanks!

I'm not even confident in what I wrote above - will update if I find a pattern of behaviour.

sinyayadynya commented 1 year ago

Actually, @shadcn started exploring a fix where it is possible to set the defaultLocale on the client: https://github.com/chapter-three/next-drupal/pull/362 (https://github.com/chapter-three/next-drupal/tree/feat/defaultLocale-for-client)

Unfortunately, I'm not sure where and how to set that defaultLocale. Would be glad if you could suggest me a way @martdavidson

shadcn commented 1 year ago

You can now install experimental releases from PRs. See https://github.com/chapter-three/next-drupal/pull/362#issuecomment-1332019345

d315kZ2T

@martdavidson Can you check out one of those releases and see if this fixes your issue?

You can now set a defaultLocale on the client.

// Will fetch JSON:API from https://drupal-site.com/es/jsonapi by default.
export const drupal = new DrupalClient(
  process.env.NEXT_PUBLIC_DRUPAL_BASE_URL,
  {
    defaultLocale: "es",
  }
)
martdavidson commented 1 year ago

@shadcn I think I just added to confusion in this thread, my issues might have been unrelated. If I run into issues again I'll check this out to see if it lines up - thanks 🙏

JohnAlbin commented 4 months ago

I just reviewed #362 and I think a better way to think about this issue is with a "default locale prefix".

For non-default locales, the locale prefix will be strings looking like "/es" or "/jp".

The default locale prefix for the default locale is an empty string "" no matter what the default locale is. But you can configure Drupal to always use a locale prefix even for the default locale.

It makes sense to me to add a defaultLocalePrefix: string config that defaults to "".

How does that sound to everyone?