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

[AdapterLuxon]: Getting error when using pt-BR locale and macro token DD #10177

Closed
2 tasks done
genepaul opened this issue Aug 30, 2023 · 6 comments · Fixed by #10400
Closed
2 tasks done

[AdapterLuxon]: Getting error when using pt-BR locale and macro token DD #10177

genepaul opened this issue Aug 30, 2023 · 6 comments · Fixed by #10400
Labels
bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module! external dependency Blocked by external dependency, we can’t do anything about it on hold There is a blocker, we need to wait support: commercial Support request from paid users

Comments

@genepaul
Copy link
Contributor

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Link to live example:

https://codesandbox.io/s/stoic-thunder-6sx5k3

Steps:

  1. Use AdapterLuxon
  2. Set the adapterLocale to pt-BR
  3. Set format on any component to DD or another macro token
  4. Notice the error

Current behavior 😯

As mentioned above, when using the AdapterLuxon, the macro token DD does not appear to work. I get the following error:

Error
MUI: The token "de" is not supported by the Date and Time Pickers.
Please try using another token or open an issue on https://github.com/mui/mui-x/issues/new/choose if you think it should be supported.

If I use other locales, like fr-FR or es-MX, it seems to work. I've only been able to reproduce this with pt or pt-BR. Interestingly, pt-PT appears to work.

Expected behavior 🤔

Not to bomb? I'd love for it to work, but at least it should maybe fallback to a default locale/format.

Context 🔦

No response

Your environment 🌎

npx @mui/envinfo
 System:
    OS: macOS 12.6.3
  Binaries:
    Node: 18.14.2 - ~/.nvm/versions/node/v18.14.2/bin/node
    Yarn: Not Found
    npm: 9.8.1 - ~/Developer/component-library-lab/node_modules/.bin/npm
  Browsers:
    Chrome: 116.0.5845.110
    Edge: 114.0.1823.67
    Firefox: 115.0.2
    Safari: 15.6.1
  npmPackages:
    @emotion/react:  11.11.1 
    @emotion/styled:  11.11.0 
    @mui/base:  5.0.0-beta.8 
    @mui/core-downloads-tracker:  5.14.2 
    @mui/icons-material:  5.14.1 
    @mui/lab:  5.0.0-alpha.137 
    @mui/material: ^5.14.0 => 5.14.2 
    @mui/private-theming:  5.13.7 
    @mui/styled-engine:  5.13.2 
    @mui/system:  5.14.1 
    @mui/types:  7.2.4 
    @mui/utils:  5.14.5 
    @mui/x-date-pickers: ^6.11.2 => 6.11.2 
    @mui/x-date-pickers-pro: ^6.11.2 => 6.11.2 
    @mui/x-license-pro: ^6.10.2 => 6.10.2 
    @types/react: ^17.0.2 => 17.0.62 
    react: ^17.0.2 => 17.0.2 
    react-dom: ^17.0.2 => 17.0.2 
    typescript: 4.9.5 => 4.9.5 

Order ID or Support key 💳 (optional)

No response

@genepaul genepaul added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 30, 2023
@zannager zannager added plan: Pro Impact at least one Pro user component: pickers This is the name of the generic UI component, not the React module! labels Aug 31, 2023
@flaviendelangle
Copy link
Member

The expanded format of DD for the locale pt-BR is d de MMM de yyyy
Where de is not a format token but is not escaped either.

@alexfauquette for me the problem is that de looks a lot like a format, so it matches isTokenStartRegExp.

The only solution I see to support it would be to totally stop throwing when we have a an unknown token and instead add it as a separator.

The following diff works and does not cause any test to fail:

--- a/packages/x-date-pickers/src/internals/hooks/useField/useField.utils.ts
+++ b/packages/x-date-pickers/src/internals/hooks/useField/useField.utils.ts
@@ -20,16 +20,11 @@ import { getMonthsInYear } from '../../utils/date-utils';
 export const getDateSectionConfigFromFormatToken = <TDate>(
   utils: MuiPickersAdapter<TDate>,
   formatToken: string,
-): Pick<FieldSection, 'type' | 'contentType'> & { maxLength: number | undefined } => {
+): Pick<FieldSection, 'type' | 'contentType'> & { maxLength: number | undefined } | null => {
   const config = utils.formatTokenMap[formatToken];
 
   if (config == null) {
-    throw new Error(
-      [
-        `MUI: The token "${formatToken}" is not supported by the Date and Time Pickers.`,
-        'Please try using another token or open an issue on https://github.com/mui/mui-x/issues/new/choose if you think it should be supported.',
-      ].join('\n'),
-    );
+    return null
   }
 
   if (typeof config === 'string') {
@@ -413,7 +408,7 @@ export const changeSectionValueFormat = <TDate>(
   newFormat: string,
 ) => {
   if (process.env.NODE_ENV !== 'production') {
-    if (getDateSectionConfigFromFormatToken(utils, currentFormat).type === 'weekDay') {
+    if (getDateSectionConfigFromFormatToken(utils, currentFormat)?.type === 'weekDay') {
       throw new Error("changeSectionValueFormat doesn't support week day formats");
     }
   }
@@ -512,10 +507,13 @@ export const splitFormatIntoSections = <TDate>(
 
   const commitToken = (token: string) => {
     if (token === '') {
-      return null;
+      return true;
     }
 
     const sectionConfig = getDateSectionConfigFromFormatToken(utils, token);
+    if (sectionConfig == null) {
+      return false
+    }
 
     const hasLeadingZerosInFormat = doesSectionFormatHaveLeadingZeros(
       utils,
@@ -566,7 +564,7 @@ export const splitFormatIntoSections = <TDate>(
       modified: false,
     });
 
-    return null;
+    return true;
   };
 
   // Expand the provided format
@@ -611,7 +609,14 @@ export const splitFormatIntoSections = <TDate>(
         escapedPartOfCurrentChar?.end === i;
 
       if (!isEscapeBoundary) {
-        commitToken(currentTokenValue);
+        const success = commitToken(currentTokenValue);
+        if (!success) {
+          if (sections.length === 0) {
+            startSeparator += currentTokenValue;
+          } else {
+            sections[sections.length - 1].endSeparator += currentTokenValue;
+          }
+        }
 
         currentTokenValue = '';
         if (sections.length === 0) {

@flaviendelangle flaviendelangle added bug 🐛 Something doesn't work support: commercial Support request from paid users and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer plan: Pro Impact at least one Pro user labels Aug 31, 2023
@alexfauquette
Copy link
Member

The DD format is expended as

  • d de MMM de yyyy for pt-BR
  • d/MMM/yyyy for pt-PT

Their are two issue with d de MMM de yyyy format:

  • On our side: we do not consider that de could be a token just because their is a token d (related to the issue about supporting format ddMMMYYYYY)
  • On luxon side with DateTime.expandFormat('DD', { locale: 'pt-BR' })} returning d de MMM de yyyy because the de should be 'de'.
    For example, DateTime.fromISO('2014-08-06T13:07:04.054').toFormat('d de MMM de yyyy', { locale: 'pt-PT', }) returns 6 6e 08 6e 2014

@alexfauquette
Copy link
Member

@flaviendelangle Your solution is to test if the token exist, and if no, put it into the separator. For now, it works without problem.

If we solve #10007 it will fail, because the parser will be able to detect the d as a token and we will get the 6 6e 08 6e 2014.

@flaviendelangle
Copy link
Member

That's true...
And if we only do it if the character after is a space?
The amount of use cases not covered would get smaller and smaller.

@LukasTy LukasTy added on hold There is a blocker, we need to wait external dependency Blocked by external dependency, we can’t do anything about it labels Sep 19, 2023
@alexfauquette
Copy link
Member

Just opened an issue in luxon to see how we can fix it: moment/luxon#1511

@flaviendelangle
Copy link
Member

https://github.com/moment/luxon/blob/5ea19fb0cc80cddd03c6af21588b5ad7dcecaa35/src/datetime.js#L937

I fear that this is intented and that they won't be able to do anything about it dues to the nature of the parsing that relis on the Intl API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module! external dependency Blocked by external dependency, we can’t do anything about it on hold There is a blocker, we need to wait support: commercial Support request from paid users
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants