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

It should throw an error when passed empty string as timezone when using moment().tz('') #1044

Open
shenxiang11 opened this issue Mar 20, 2023 · 1 comment
Labels
Milestone

Comments

@shenxiang11
Copy link

Moment-timezone version which you use:

Version: latest

Note: many issues are resolved if you just upgrade to the latest version

Issue description:

The function returns undefined, it's not conform to the interface in index.d.js.
I think it should change themoment.Moment to moment.Moment | undefined
or throw an error like we pass a timezone which is not exist?

interface Moment {
        tz(): string | undefined;
        tz(timezone: string, keepLocalTime?: boolean): moment.Moment; // or should be moment.Moment | undefined ?
        zoneAbbr(): string;
        zoneName(): string;
}
@gilmoreorless
Copy link
Member

First up, please don't list "latest" in the version field, as it becomes incorrect very quickly. For example, a new version has already been published today. To diagnose issues, we need exact version numbers.

From looking at the code, it seems moment().tz('') is treated exactly the same as moment().tz(), returning string | undefined. This is because of a check of if (zone) which treats the empty string the same as undefined.

I agree that this behaviour doesn't match the type definitions, and isn't quite correct. But fixing it would technically be a breaking change, as code that worked previously could start throwing errors. I'm going to link this to the proposed breaking changes in #1039.

@gilmoreorless gilmoreorless added this to the 1.0.0 milestone Apr 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants