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

bug(LuxonDateAdapter): LuxonDateAdapter.parse() returns valid date even though none of the given input date formats is matched #25590

Open
1 task
TardigradeX opened this issue Sep 7, 2022 · 3 comments
Labels
area: material/datepicker P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@TardigradeX
Copy link

Is this a regression?

  • Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

The "parse" function in the LuxonDateAdapter tries to parse any input as a iso8601 date before it matches the given input formats.

For Example, if I declare ['d.L.yy', 'd-L-yy'] as valid input formats, I would expect that only dates using the separators "." and "-"
are valid input dates.
But due to the iso8601 parse a simple number input (eg. 12) will return a valid date, most times today.
You can see the iso8601 parse and return on line 162 of luxon-date-.adapter.ts
Where as the formats are checked later on line 174.

  parse(value: any, parseFormat: string | string[]): LuxonDateTime | null {
    const options: LuxonDateTimeOptions = this._getOptions();

    if (typeof value == 'string' && value.length > 0) {
      const iso8601Date = LuxonDateTime.fromISO(value, options);

      if (this.isValid(iso8601Date)) {
        return iso8601Date; // This return is without any parseFormat check !!!!!!
      }

      const formats = Array.isArray(parseFormat) ? parseFormat : [parseFormat];

      if (!parseFormat.length) {
        throw Error('Formats array must not be empty.');
      }

      for (const format of formats) {
        const fromFormat = LuxonDateTime.fromFormat(value, format, options); // Here are the formats checked

        if (this.isValid(fromFormat)) {
          return fromFormat;
        }
      }

      return this.invalid();
    } else if (typeof value === 'number') {
      return LuxonDateTime.fromMillis(value, options);
    } else if (value instanceof Date) {
      return LuxonDateTime.fromJSDate(value, options);
    } else if (value instanceof LuxonDateTime) {
      return LuxonDateTime.fromMillis(value.toMillis(), options);
    }

    return null;
  }

This parsing without any regard for the given input parseFormat seems to be incorrect.

Reproduction

There is test in luxon-date-adapter,spec.ts on line 310 which shows the behaviour . The input format is LL/dd/yyyy, but just a number is given.

Expected Behavior

The test mentioned above should return an invalid date, because the given parseFormat is not matched.

Actual Behavior

The function returns a valid ISO date, based on the number.

Environment

"@angular/animations": "12.2.16",
"@angular/cdk": "12.2.13",
"@angular/common": "12.2.16",
"@angular/compiler": "12.2.16",
"@angular/core": "12.2.16",
"@angular/forms": "12.2.16",
"@angular/material": "12.2.13",
"@angular/material-luxon-adapter": "12.2.13",
"@angular/platform-browser": "12.2.16",
"@angular/platform-browser-dynamic": "12.2.16",
"@angular/router": "12.2.16",
"luxon": "2.3.0",

Above are the versios we are using in our code, but the code pasted in the description is from the current @angular/material-luxon-adapter, so the bug is still there.

@TardigradeX TardigradeX added the needs triage This issue needs to be triaged by the team label Sep 7, 2022
@devcrev
Copy link

devcrev commented Nov 23, 2022

@TardigradeX
I have been waiting for a resolution on this. I ended up copying the needed files to my project and then I commented out the ISO logic in the beginning of the parse function. All is well for the moment or should I say all is well for the luxon 😊.

@Delubear
Copy link

Would like to see this fixed or at least a workaround. Users entering "12" into my date field should not count as "valid" input and then they end up with todays date.

@Delubear
Copy link

Delubear commented May 16, 2023

For anyone else, ended up removing this behaviour like so:

import { LuxonDateAdapter } from '@angular/material-luxon-adapter';
import { DateTime } from 'luxon';

export class CustomLuxonAdapterModule extends LuxonDateAdapter {
  private _useUTCCustom: boolean;

  constructor(dateLocale: any, options: any) {
    super(dateLocale, options);
    this._useUTCCustom = !!options?.useUtc;
  }

  parse(value: any, parseFormat: any) {
    const options = this._getOptionsCustom();
    if (typeof value == 'string' && value.length > 0) {
      // -- PROBLEM CODE HERE
      // const iso8601Date = DateTime.fromISO(value, options);
      // if (this.isValid(iso8601Date)) {
      //     return iso8601Date;
      // }
      const formats = Array.isArray(parseFormat) ? parseFormat : [parseFormat];
      if (!parseFormat.length) {
        throw Error('Formats array must not be empty.');
      }
      for (const format of formats) {
        const fromFormat = DateTime.fromFormat(value, format, options);
        if (this.isValid(fromFormat)) {
          return fromFormat;
        }
      }
      return this.invalid();
    } else if (typeof value === 'number') {
      return DateTime.fromMillis(value, options);
    } else if (value instanceof Date) {
      return DateTime.fromJSDate(value, options);
    } else if (value instanceof DateTime) {
      return DateTime.fromMillis(value.toMillis(), options);
    }
    return null;
  }

  /** Gets the options that should be used when constructing a new `DateTime` object. */
  private _getOptionsCustom() {
    return {
      zone: this._useUTCCustom ? 'utc' : undefined,
      locale: this.locale,
    };
  }
}

Then in my app.module.ts, I added the following under "providers":

{
      provide: DateAdapter,
      useClass: CustomLuxonAdapterModule,
      deps: [MAT_DATE_LOCALE, MAT_LUXON_DATE_ADAPTER_OPTIONS],
    },

@amysorto amysorto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent area: material/datepicker and removed needs triage This issue needs to be triaged by the team labels May 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: material/datepicker P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests

4 participants