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

Rename locals to locales #441

Closed

Conversation

woodcockjosh
Copy link

@woodcockjosh woodcockjosh commented Jul 28, 2020

Possible solution to
DefinitelyTyped/DefinitelyTyped#46328 (comment)
#440

@types/i18n would need to be so that

declare namespace Express {
    interface Request extends i18nAPI {
        languages: string[];
        regions: string[];
        language: string;
        region: string;
    }

    interface Response extends i18nAPI {
        locales: i18nAPI;
    }
}

in order to prevent conflict

Alternatively we can just remove strict type in @types/i18n for locals

declare namespace Express {
    interface Request extends i18nAPI {
        languages: string[];
        regions: string[];
        language: string;
        region: string;
    }

    interface Response extends i18nAPI {
        locals: any;
    }
}

I don't see how that helps with applications that want to be strongly typed but its a possible solution.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.773% when pulling 31a048c on woodcockjosh:rename-locals-to-locales into fb9aa22 on mashpie:master.

@mashpie
Copy link
Owner

mashpie commented Jul 28, 2020

dear... in short:

  • res.locals -> property of express.js
  • res.locales -> property of i18n
  • res.locals.locales -> property of i18n

won't merge / can't merge in fact - sorry for so many search & replace

as far as I get it:

    interface Response extends i18nAPI {
        locals: any;
    }

SHOULD NOT be part of i18n types as locals is a property of express response object

while:

    interface Response extends i18nAPI {
        locales: i18nAPI;
    }

COULD be (didn't check i18nAPI types in detail though)

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.

None yet

3 participants