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

TypeScript compile error: Duplicate identifier 'Options' #3

Closed
klanchman opened this issue Dec 21, 2018 · 13 comments
Closed

TypeScript compile error: Duplicate identifier 'Options' #3

klanchman opened this issue Dec 21, 2018 · 13 comments

Comments

@klanchman
Copy link

klanchman commented Dec 21, 2018

Seeing an issuee when trying to use date-fns-tz in a TypeScript project.

Reproduction

Library Version
create-react-app 2.1.1
date-fns 2.0.0-alpha.26
date-fns-tz 0.1.2
  • Create a new project with create-react-app <projectName> --typescript
  • Open App.tsx and import date-fns and date-fns-tz, e.g:
import { addMinutes } from "date-fns";
import { utcToZonedTime } from 'date-fns-tz'
  • Run npm start / yarn start

Expected

App runs

Actual

Compile error

[snip]node_modules/date-fns-tz/typings.d.ts
Type error: Duplicate identifier 'Options'.  TS2300

    27 | // Type Aliases
    28 |
  > 29 | type Options = {
       |      ^
    30 |   weekStartsOn?: 0 | 1 | 2 | 3 | 4 | 5 | 6
    31 |   firstWeekContainsDate?: 1 | 2 | 3 | 4 | 5 | 6 | 7
    32 |   additionalDigits?: 0 | 1 | 2

If I manually remove Options, OptionsAliased, Locale, LocaleAliased from node_modules/date-fns-tz/typings.d.ts, then it compiles, and I think the functions from date-fns-tz are still typed as expected?

@motin
Copy link

motin commented Dec 30, 2018

This is being fixed in https://github.com/marnusw/date-fns-tz/commits/fixTypescript, but it is not ready yet. Tried npm install --save marnusw/date-fns-tz#fixTypescript but it didn't work.

Also tried cloning this repo, switching to the fixTypescript branch and running the following:

npm install
chmod +x scripts/build/*
npm run build
export PACKAGE_OUTPUT_PATH=./path/to/your/project/node_modules/date-fns-tz
npm run package

... but encountered multiple issues during build and packaging and compiling, so I assume the branch reflects work in progress.

@marnusw
Copy link
Owner

marnusw commented Dec 30, 2018

Thanks for the detailed bug report. Yes, work in progress, just a little slow over the holiday period. I'll keep you posted.

@marnusw
Copy link
Owner

marnusw commented Jan 1, 2019

I have published 1.0.0-beta.1 where the reported issue appears to be fixed. Using

import utcToZonedTime from 'date-fns-tz/utcToZonedTime'

works correctly now, but the named import

import { zonedTimeToUtc } from 'date-fns-tz'

errors with Type error: Module '"date-fns-tz"' has no exported member 'zonedTimeToUtc'. TS2305. I've not worked with Typescript all that much so I'm wondering whether either of you have some insight into why this happens? I've compared the typings file with the one used by date-fns and I can't spot any issues. If you don't know I might ask the date-fns maintainers if they would mind helping out.

@klanchman
Copy link
Author

Thanks for the update @marnusw!

I looked at the issue you're talking about, and found an explanation and a potential fix here.

I tried changing the locale?: Locale line to locale?: import('date-fns').Locale, and removed the import at the top, and that seems to have settled it down for me.

Unfortunately I'm not well-versed in creating declarations files either, so I'm not sure if there's a better way to fix it. It seems like this could get clunky if you needed a type from date-fns in multiple places throughout the declaration file...

@marnusw
Copy link
Owner

marnusw commented Jan 1, 2019

Thank you @klanchman!

It looks like all Typescript issues have been resolved in 1.0.0. If so this issue can be closed.

@klanchman
Copy link
Author

Checked it out in my full project and seems good. Thank you! 😄

@kamranayub
Copy link

kamranayub commented Jan 14, 2019

@klanchman It appears this import('date-fns').Locale is not supported in TS 2.8.3 (which is the create-react-app-typescript fork version). I can explore what typings might be backwards compatible to submit a PR potentially.

edit: Here's an example typings file that works fine in TS 2.8 and 3.2:

declare module 'date-fns-tz' {
  import { Locale } from "date-fns"

  export type OptionsWithTZ = {
    weekStartsOn?: 0 | 1 | 2 | 3 | 4 | 5 | 6
    firstWeekContainsDate?: 1 | 2 | 3 | 4 | 5 | 6 | 7
    additionalDigits?: 0 | 1 | 2
    timeZone?: string
    locale?: Locale
    includeSeconds?: boolean
    addSuffix?: boolean
    unit?: 'second' | 'minute' | 'hour' | 'day' | 'month' | 'year'
    roundingMethod?: 'floor' | 'ceil' | 'round'
    awareOfUnicodeTokens?: boolean
  }

  export function toDate(
    argument: Date | string | number,
    options?: OptionsWithTZ
  ): Date

  export function utcToZonedTime(
    date: Date | string | number,
    timeZone: string,
    options?: OptionsWithTZ
  ): Date

  export function zonedTimeToUtc(
    date: Date | string | number,
    timeZone: string,
    options?: OptionsWithTZ
  ): Date
}

declare module 'date-fns-tz/toDate' {
  import { toDate } from 'date-fns-tz'
  export default toDate
}

declare module 'date-fns-tz/zonedTimeToUtc' {
  import { zonedTimeToUtc } from 'date-fns-tz'
  export default zonedTimeToUtc
}

declare module 'date-fns-tz/utcToZonedTime' {
  import { utcToZonedTime } from 'date-fns-tz'
  export default utcToZonedTime
}

@klanchman
Copy link
Author

Ah, I didn't notice that. I'm also using the create-react-app-typescript fork, but with TS 3.1.

Your example deviates quite a bit from how the definition file is set up currently. It also doesn't account for the fp, esm, and esm/fp modules, all of which also use OptionsWithTZ. The way it's structured now lines up pretty closely with how date-fns is doing it too, which I imagine was the intent.

That said, it might not be a problem to take your example and just duplicate the OptionsWithTZ definition across each module. I haven't looked closely at how the typings are being generated to know for sure. @marnusw would know more about that 😄

@marnusw
Copy link
Owner

marnusw commented Jan 14, 2019

This should be simple enough to change. I'll take a look when I can. Thank you for investigating.

@marnusw marnusw reopened this Jan 14, 2019
@marnusw
Copy link
Owner

marnusw commented Jan 21, 2019

@kamranayub when I change the typings file to

declare module 'date-fns-tz' {
  import { Locale } from "date-fns"

  export type OptionsWithTZ = {
    weekStartsOn?: 0 | 1 | 2 | 3 | 4 | 5 | 6
    firstWeekContainsDate?: 1 | 2 | 3 | 4 | 5 | 6 | 7
    additionalDigits?: 0 | 1 | 2
    timeZone?: string
    locale?: Locale
    includeSeconds?: boolean
    addSuffix?: boolean
    unit?: 'second' | 'minute' | 'hour' | 'day' | 'month' | 'year'
    roundingMethod?: 'floor' | 'ceil' | 'round'
    awareOfUnicodeTokens?: boolean
  }
}

Then the other module declarations lower down break because OptionsWithTZ is no longer globally defined. It looks like this can be fixed by adding import { OptionsWithTZ } from "date-fns-tz" to each of the other module declaration blocks.

But what about the final section that defines // dateFns Global Interface with interface dateFns { ... }. I'm not familiar enough with TypeScript to know how important that section is or how it will be used. If it's not important it can be removed and we should have a working solution, if not, we need to find a way to make OptionsWithTZ available to that interface definition which is also compatible with all the relevant TS versions. Any thoughts?

@marnusw
Copy link
Owner

marnusw commented Feb 2, 2019

I have implemented @kamranayub 's suggestion in 1.0.7-beta.1. If you would try it out in your respective projects in the coming days and let me know if it is compatible with all relevant versions of TS that would be appreciated. Then I'll do a full release.

@klanchman
Copy link
Author

Looks to be working for me with TypeScript 3.1 and 3.3 🙂

@marnusw
Copy link
Owner

marnusw commented Feb 9, 2019

Released in 1.0.7.

@marnusw marnusw closed this as completed Feb 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants