-
Notifications
You must be signed in to change notification settings - Fork 76
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
Convert to Typescript? #141
Comments
All for typescript, considered it myself once or twice for this repo but didn't find the time yet. I haven't used either of the boilerplates referenced, but I'd say go with that and let's get existing functionality + jest tests working - then I could take a look at re-adding cypress tests pre-release. |
I'm giving it a shot. |
@DJTB Do you know anything about |
Is it exported/exposed in the final package? I know I've used it before,
not sure of the context (perhaps with my react-furi package). Afk at the
moment, but I'll check more thoroughly later today.
…On Sat, 4 Jun 2022, 14:30 Mims H. Wright, ***@***.***> wrote:
Do you know anything about tokenize( input, {detailed: true})? It doesn't
appear to be used internally at all except in a few tests. Maybe we can
make it a separate function to simplify the type checks.
—
Reply to this email directly, view it on GitHub
<#141 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABI25P3SV4ZMF2AGKILK4MDVNLLYFANCNFSM5XOBX56A>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Turns out tsdx is what I started with and it's no longer being maintained. Going to see if I can find some other alternative. |
If I'm being honest (with myself) i kind of have a ton of stuff to do right now and i think i took this on as a form of procrastination. I might need to step away for a while and not touch it until i actually have the free time. |
No problem, I know the feeling! |
I did do a fair amount of rework to typescript so if you start this process, let me know and I'll send you what I have. the main issue was just as i feared, wrestling with the config files. JS builds are really too complicated. |
Hey, I've been wanting to help out with a potential TS conversion of react-furi, but then realized that it probably makes sense to convert this project first. I've had quite some experience now converting to TS without ts-dx, but do agree that Vite might be a nice way to go, since this project already has Rollup :) I'm gonna give it a go and let y'all know if there's progress. |
Awesome, any contributions are more than welcome! |
@Naoto-Ida @DJTB |
…compiler for type definitions As part of adding Typescript, a number of dependencies had to be updated. All @babel packages and @rollup packages were updated. Additionally prettier-eslint-cli had to be updated due to a typing issue in the dependency tree. This also meant that eslint also had to be updated to maintain compatibility. Due to these package changes the minumum node devEngines version has been bumped to 16. A tsconfig.json file has been added, as well as renaming the base index file from index.js to index.ts. Adds packages to enable WaniKani#141
…e definition output constants.js has been changed to a typescript file to enable the exporting of the DefaultOptions type. This is then used by various function definitions, which provides type checking for the parameters. Regexp has been changed to RegExp as Regexp was resolving to the any type. Other parameters defined as Object have been specified with types to match their expected input. TOKEN_TYPES's export was removed as it was unused and was being generated into the d.ts file. Resolve external typings for WaniKani#141
…les and typings Added better-docs to piggyback off of its typescript plugin. Updated instances of string to String and boolean to Boolean. This matches the rest of the existing documentation. Typescript types are unaffected by these changes and automatically get converted to the correct typescript type. Changed instances of [index: string]: string to Object.<string, string> JSDoc type. This gets converted in typescript, but the previous syntax was not supported in JSDocs. WaniKani#141
Hey all! I just wanted to let people know that typescript is now available in the repo. In the end I opted for a smaller change than moving to a new build system or organisation of the library, this way we can more incrementally move things over when we're able to. I see that @mimshwright and @Naoto-Ida have mentioned making some changes for typescript. It should be possible to move stuff over if you think it'll be useful! |
Hi, is this normal ? const tokens = tokenize(input, { detailed: true }) as Array<{
type: string
value: string
}> TS2345: Argument of type { detailed: true; } is not assignable to parameter of type
{ compact: boolean | undefined; detailed: boolean | undefined; }
Property compact is missing in type { detailed: true; } but required in type
{ compact: boolean | undefined; detailed: boolean | undefined; } Shouldn't you be able to put only one of them since they do 2 differents things based on
|
This appears to have been an oversight when type annotations were added. Note this is only a type error, the function can happily accept an object with only one of the keys. |
Mentioned by @DJTB in #139
Hey, what if we converted to ts? Would there be interest?
I took a quick look at the effort involved. It doesn't look like converting the code would take long at all and I'd be happy to do it. However, hacking together the right config files to make it compile might and I'd rather not do that. Perhaps it might even be more prudent to try copying the source to a new ts-dx or vite boilerplate.
The text was updated successfully, but these errors were encountered: