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

support intl.get(key) type check #83

Closed
wants to merge 1 commit into from

Conversation

hardfist
Copy link
Contributor

Change-Id: I66f70281e5f074cc83204e4aff64cf05c57265a2

Change-Id: I66f70281e5f074cc83204e4aff64cf05c57265a2
@CLAassistant
Copy link

CLAassistant commented Sep 16, 2018

CLA assistant check
All committers have signed the CLA.

@hardfist
Copy link
Contributor Author

hardfist commented Sep 16, 2018

fix issue #82

it now supports checks the value pass to intl.get(key) ,key is valid if it's translation's key.

2018-09-16 2 17 35

and we can run tsc to check our code;
2018-09-16 2 18 42

@jhaenchen
Copy link
Contributor

@cwtuan Can we merge? i love this <3

@jhaenchen
Copy link
Contributor

I still love this and would still love to see it merged!

@cwtuan
Copy link
Collaborator

cwtuan commented Oct 23, 2018

This PR makes existing project shows error if they doesn't have the definition file.

This also force users to enable the ts type checking even if they don't want this feature.

@hardfist
Copy link
Contributor Author

hardfist commented Oct 23, 2018

@cwtuan , Yes, It's a breaking change, and I think It's impossible to implement key check without introducing breaking change, and the fixing is easy for old project.
for js project there is not problem,
for ts project just adding this declaration.

declare module 'react-intl-universal' {
  export interface Message extends Record<string, string> {}
}

@cwtuan
Copy link
Collaborator

cwtuan commented Oct 23, 2018

Yes, we could ask user to add that declaration, but it may introduce more difficulty to newcomer.
Is there other implementation to make this feature disabled by default?

@hardfist
Copy link
Contributor Author

@cwtuan Yes, we could disable this feature by default by using generic parameter default

 /**
     * Get the formatted message by key
     * @param {string} key The string representing key in locale data file
     * @returns {string} message
     */
    export function get<T = Key>(key:T): string;

in which key type check is disabled unless you pass type to generic
2018-10-24 10 32 30
But I thinks it's kind of annoying to pass type every time

@jhaenchen
Copy link
Contributor

Maybe we could add another .get function signature or get function? Like getSafely?

@cwtuan
Copy link
Collaborator

cwtuan commented Oct 25, 2018

@jhaenchen
Another signature may make user confused.

@hardfist hardfist closed this Aug 27, 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

Successfully merging this pull request may close these issues.

4 participants