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 - Reconsider RequestInfo class name as the name is built-in in the language #551

Closed
Tracked by #1049
nikithauc opened this issue Aug 30, 2021 · 3 comments · Fixed by #559
Closed
Tracked by #1049
Assignees
Labels
Csharp Pull requests that update .net code fixed Go Java Ruby TypeScript Pull requests that update Javascript code
Milestone

Comments

@nikithauc
Copy link
Contributor

nikithauc commented Aug 30, 2021

RequestInfo is defined in the TypeScript library as follows:

Please check my question on StackOverFlow for more info: https://stackoverflow.com/questions/68970860/typescript-using-predefined-names-for-custom-class-names.

The following prompts a Duplicate Identifier error :

class RequestInfo{}

However, adding the export keyword export class RequestInfo removes the error. I learnt from the TS community that reusing a built-in name is not recommended.

Additionally, use HTTPContext or MiddlewareContext which would be the conventional term for the purpose of this class. Examples: GraphRequestContext in Graph dot net core,
RequestContext -> https://github.com/microsoftgraph/msgraph-sdk-design/blob/master/middleware/RequestContext.md,
HTTPContext - https://docs.microsoft.com/en-us/dotnet/api/system.web.httpcontext?view=netframework-4.8

AB#10917

@nikithauc nikithauc added the TypeScript Pull requests that update Javascript code label Aug 30, 2021
@baywet baywet added Csharp Pull requests that update .net code Go Java Ruby labels Aug 30, 2021
@baywet baywet self-assigned this Aug 30, 2021
@baywet baywet added this to the Beta milestone Aug 30, 2021
@baywet
Copy link
Member

baywet commented Aug 30, 2021

Thanks a lot! that's a very good catch, I'm not sure how I didn't catch it earlier.
I suggest that we simply rename it to RequestInformation, using abbreviations when naming something is generally thrown upon anyway.
Thoughts?

@darrelmiller
Copy link
Member

I'm ok with RequestInformation. I don't feel strongly about it.

@baywet
Copy link
Member

baywet commented Aug 30, 2021

fixed via #559

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Csharp Pull requests that update .net code fixed Go Java Ruby TypeScript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants