catamphetamine / react-phone-number-input

React component for international phone number input
http://catamphetamine.gitlab.io/react-phone-number-input/
MIT License
915 stars 193 forks source link

fix: add generated TypeScript definitions for locales #411

Closed iiroj closed 1 year ago

iiroj commented 1 year ago

This PR adds a new function to runnable/generate-locale-exports.js that outputs a stub type definition file for every locale. The emitted files are auto-discovered by TSC because they're the same name.

For my use-case this fixes importing locales, but not sure if the exact type itself is 100% correct.

What do you think @catamphetamine?

catamphetamine commented 1 year ago

@iiroj Oh, right. Thx. Forgot to add TypeScript definitions for those locales. I've based my change on your proposed changes: https://github.com/catamphetamine/react-phone-number-input/commit/91b305f80813a5b9f5c20f4ddea19488957373c5

Published react-phone-number-input@3.2.5.

On my local machine, it didn't like the usual import for some reason (perhaps an outdated version of TS compiler as I don't use it):

index.ts:29:32 - error TS2307: Cannot find module 'react-phone-number-input/locale/ru' or its corresponding type declarations.

29 import PhoneInputLocaleRu from 'react-phone-number-input/locale/ru'

But when changed react-phone-number-input/locale/ru to react-phone-number-input/locale/ru.json the error disappeared, so I assume it's working.

iiroj commented 1 year ago

Hey, thanks for the update!

I think the reason that react-phone-number-input/locale/ru doesn't work but react-phone-number-input/locale/ru.json does, is that the syntax in package.json might be wrong, and that's why TSC can only find the type definitions when the name matches exactly, for example locale/ru.js -> locale/ru.d.ts.

If you look at this:

{
  "exports": {
    "./locale/ar": {
       "types": "./locale/ar.json.d.ts",
       "import": "./locale/ar.json.js",
       "require": "./locale/ar.json"
     }
  }
}

And compare it to the release notes of TypeScript 4.7: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-7.html#packagejson-exports-imports-and-self-referencing

It looks like it should be:

{
    "exports": {
        "./locale/ar": {
            "import": {
                "types": "./locale/types.d.ts",
                "default": "./locale/ar.json.js"
            },
            "require": {
                "types": "./locale/types.d.ts",
                "default": "./locale/ar.json"
           }
     }
  }
}

Personally I couldn't get this to work, however, hence my PR which added duplicate definition files, ru.d.ts and ru.json.d.ts in this case.

catamphetamine commented 1 year ago

It looks like it should be:

No, the example they've provided there is just to illustrate how to use different "typings" for ES Modules vs CommonJS.

Here's a correct example: https://devblogs.microsoft.com/typescript/announcing-typescript-4-5-beta/#packagejson-exports-imports-and-self-referencing

iiroj commented 1 year ago

Thanks, TIL! In any case, if you emit both ${locale}.d.ts and ${locale}.json.d.ts it might help.