Convert to typescript

[Pandapip1]

Also fixes a couple of bugs I found because typescript was throwing an error.

See: PrismarineJS/prismarine-contribute#7

[Pandapip1]

CC @rom1504

[socket-security]

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages	Version	New capabilities	Transitives	Size	Publisher
@tsconfig/recommended	1.0.3	None	+0	2.37 kB	typescript-deploys
@types/node-fetch	2.6.10	None	+8	4.37 MB	types
@types/mocha	10.0.6	None	+0	95.6 kB	types
@types/node	20.11.0	None	+1	4.05 MB	types
typescript	5.3.3	filesystem	+0	32 MB	typescript-bot

[Pandapip1]

FWIW node-fetch should also be phased out.

[rom1504]

This looks ok to me. Maybe a bit too many Any?

What do you recommend instead of node fetch?

This package is maintained mostly by @extremeheat and @LucienHH so they'll have the final word on whether we do this

[Pandapip1]

What do you recommend instead of node fetch?

NodeJS has a native fetch function

[rom1504]

I'm looking more and some points

• https://github.com/Pandapip1/prismarine-auth/blob/typescriptify/index.js why is this file still js
• https://github.com/Pandapip1/prismarine-auth/tree/typescriptify/examples should examples be ts ?

[rom1504]

About dropping node fetch sgtm, this can be done in another PR, since that's a pretty trivial change

[Pandapip1]

https://github.com/Pandapip1/prismarine-auth/blob/typescriptify/index.js why is this file still js

Must've missed it.

https://github.com/Pandapip1/prismarine-auth/tree/typescriptify/examples should examples be ts ?

Sure, if that's something that's wanted.

Maybe a bit too many Any?

Definitely too many anys to my liking, but for a lot of them I am too unfamilliar with the data structure to write proper typings.

EDIT: There are actually quite a few typings in the global .d.ts file. I'll look into moving those throught the project where they make sense.

[zardoy]

Definitely too many anys to my liking, but for a lot of them I am too unfamilliar with the data structure to write proper typings.

What do you think of using "noImplicitAny": false to not get annoyed with defining a lot of anys? When I migrate I always do that personally so you don't get overwhelmed with these errors. No saying that you should use this option tho

Btw if you encounter modules that don't have typings and @types/ package you can try maxNodeModuleJsDepth.

AFAIK standard doesn't work with ts (I suppose its too old), try https://github.com/standard/ts-standard

Also outDir is missing... don't forget to enable sourceMap so debugger can map paths to source

Also what do you think of adding useUnknownInCatchVariables?

And also lets add noFallthroughCasesInSwitch which can be handy I think

And doesn't adding module: Node16 mean that output format will be changed to esm, which is breaking?

[Pandapip1]

What do you think of using "noImplicitAny": false to not get annoyed with defining a lot of anys? When I migrate I always do that personally so you don't get overwhelmed with these errors. No saying that you should use this option tho

anys look bad. Every time you see one, it's a reminder to fix it.

Btw if you encounter modules that don't have typings and @types/ package you can try maxNodeModuleJsDepth.

That's not needed. All modules have types.

AFAIK standard doesn't work with ts (I suppose its too old), try https://github.com/standard/ts-standard

It doesn't always work. For this project, it does. For now, I'm making as few changes as possible.

Also outDir is missing... don't forget to enable sourceMap so debugger can map paths to source

I've gitignored the output.

Also what do you think of adding useUnknownInCatchVariables?
And also lets add noFallthroughCasesInSwitch which can be handy I think

Not familliar with those options. Mind explaining?

And doesn't adding module: Node16 mean that output format will be changed to esm, which is breaking?

The tests pass, which to me seems like it isn't breaking anything

[rom1504]

Sure, if that's something that's wanted.

Not sure it's wanted. That's probably one of the main problem with typescript. It will create confusion

[Pandapip1]

Not sure it's wanted. That's probably one of the main problem with typescript. It will create confusion

I can change the file extensions and make no modifications

[zardoy]

useUnknownInCatchVariables

Its pretty straightforward. Changes type to be any in catch of try/catch blocks. https://www.typescriptlang.org/tsconfig#useUnknownInCatchVariables

noFallthroughCasesInSwitch

Hm it turns out I was using https://eslint.org/docs/latest/rules/no-fallthrough instead of this one. But this is almost the same, helps to catch missed breaks, can skip adding this if you don't think its needed.

I've gitignored the output.

Oh, now I see, so you don't output them into another dir. But didn't the format of js change to esm? that would be breaking once published to npm since its currently cjs

[rom1504]

My point about examples is I don't know what we should do and I'm not sure there is a good solutions.

If they're in JavaScript then people won't be able to contribute back their code to the module since we are then recommending JavaScript and they will be using that.
If they're in typescript then users and maintainers of other packages will be confused how to use this in JavaScript

If we have examples in both languages it's pretty bad duplication

Maybe the less bad solution is converting them to typescript, but not sure.

[zardoy]

If they're in typescript then users and maintainers of other packages will be confused how to use this in JavaScript

That's why most modern docs websites have that JS/TS toggle, where js version is generated automatically by removing the types. My personal opinion is that they should be in ts so its easier for ts users. Also, it doesn't look like current examples need anything ts-specific

[LucienHH]

In this state does this actually build at the moment, seem to get build errors on my end?

[LucienHH]

xbl through to doTitleAuth shouldn't be marked as public properties, they're internal and are not part of the API so shouldn't be documented in types.

[LucienHH]

afaik fetch### shouldn't be part of the Authflow options

[LucienHH]

The response in this callback can vary depending on if MsaTokenManager is being used or LiveTokenManager.

https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/603098e62124b90c13dcd6e57a7d83d95cc07ce8/lib/msal-common/src/response/DeviceCodeResponse.ts#L15

Live - Returns ServerDeviceCodeResponse
Msa - Returns DeviceCodeResponse

[LucienHH]

deviceType can be of 'Nintendo' | 'iOS' | 'Win32'

[LucienHH]

Also yeah a lot of skipped types here if this refactor is to be worth it we should aim to have a fully typed API.

[Pandapip1]

Also yeah a lot of skipped types here if this refactor is to be worth it we should aim to have a fully typed API.

That's what I'm working on ;)

[zardoy]

btw doing a similar thing in flying squid: PrismarineJS/flying-squid#651

and standard crashes as soon as it sees declare (or any other ts-specific thing), tried to ts-standard but it introduces too much complexity to the project, any alts?

[Pandapip1]

and standard crashes as soon as it sees declare (or any other ts-specific thing), tried to ts-standard but it introduces too much complexity to the project, any alts?

A custom eslint config?