fix: remove setting defaults in ssr from useDefaultLocale

[herkulano]

Closes #7344

✅ Pull Request Checklist:

• Included link to corresponding React Spectrum GitHub Issue.
• Added/updated unit tests and storybook for this change (for new code or code which already has tests).
• Filled out test instructions.
• Updated documentation (if it already exists for this component).
• Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

[snowystinger]

So I don't think we want to completely remove this. Instead, we want to split the I18nProvider's internals into two components, one which calls the useDefaultLocale and one which doesn't. This is how to conditionally call hooks since they can't be in an if statement.

Which component we call inside the I18nProvider should be determined by if the locale prop is passed to the I18nProvider.

see proposal here: #7344 (comment)

Side note, if you've done it this way instead intentionally, can you explain your decision?

[herkulano]

Sorry, made the changes to split the provider.

[herkulano]

Although the currentLocale is set to the default en-US even on SSR:

let locale = typeof window !== 'undefined' && window[localeSymbol]
    // @ts-ignore
    || (typeof navigator !== 'undefined' && (navigator.language || navigator.userLanguage))
    || 'en-US';

So setting a default on SSR in the useDefaultLocale feels unnecessary.

[herkulano]

After trying the provider split I realized that we need to re-render on languagechange and removing the useDefaultLocale() hook entirely from the provider breaks components like NumberField that require a re-render when changing language: https://app.circleci.com/pipelines/github/adobe/react-spectrum/27484/workflows/dad88c6d-a527-49a0-bad5-02a412732311/jobs/470813

I now believe the initial approach was correct and we don't need to split the provider as the useDefaultLocale() is necessary even if the locale is set in the provider. I believe the extra re-render comes from returning the default locale on SSR.

[snowystinger]

Thanks for updating the PR. NumberField needing a re-render doesn't seem desirable, that sounds like a bug. Can you see why it needs an extra render?

I quickly had a look through the first failing test, it looks like the inputValue should be correct, so I'm not sure why the test is failing

[herkulano]

@snowystinger looks good, the only question I still have is if we really need to set the conditional default on SSR since it's already set by the currentLocale? If yes, then it's ready to merge, if not I'll remove it.

[snowystinger]

I'm sorry, I don't quite follow. What are you proposing?

[herkulano]

I propose removing this conditional return:

  // We cannot determine the browser's language on the server, so default to
  // en-US. This will be updated after hydration on the client to the correct value.
  if (isSSR) {
    return {
      locale: 'en-US',
      direction: 'ltr'
    };
  }

Since it's already set by currentLocale with the getDefaultLocale():

let [defaultLocale, setDefaultLocale] = useState(currentLocale);

as the locale in getDefaultLocale() fallsback to en-US:

let locale = typeof window !== 'undefined' && window[localeSymbol]
    // @ts-ignore
    || (typeof navigator !== 'undefined' && (navigator.language || navigator.userLanguage))
    || 'en-US';

[snowystinger]

Thanks, I see what you're saying now. Yes it would appear we could remove it, but also, I kind of like how explicit it is about what happens in the SSR case. I think leave it for now unless there is a compelling reason to get rid of it.

[herkulano]

Alright, all done then.