search

speedcontrol / nodecg-speedcontrol

NodeCG bundle to help facilitate the running of speedrunning marathons, including overlays.
MIT License
45 stars 34 forks source link

Update types for NodeCG 2.0 #139

Closed EwanLyon closed 1 year ago

EwanLyon commented 1 year ago

Updated types and setup for the new types coming in to NodeCG 2.0 (https://github.com/nodecg/nodecg/pull/546). Includes some package upgrades for typescript and its linters.

I think the webpack config might need to be updated as the transpileOnly option had to be set to false which the comment indicates that ForkTsCheckerWebpackPlugin won't do any type checking.

The only issue I still have with this is that there is the _updateClosingReasonConfirmed() function used on dialog boxes which I can't find in NodeCG and isn't on the types. The dialog has had to be cast as an any before using but I am unsure of the function in general, so I left it as is.

Since NodeCG 2.0 is still in development this shouldn't be merged yet.

zoton2 commented 1 year ago

Tested the code myself and seems good, this is all new stuff to me too so obviously not sure about it all but it should be good. Left a few comments about stuff I'm not sure of.

I think the webpack config might need to be updated as the transpileOnly option had to be set to false which the comment indicates that ForkTsCheckerWebpackPlugin won't do any type checking.

Do you know why you had to set it to false? Was something failing? I should give it a try myself so will do that in a bit.

The only issue I still have with this is that there is the _updateClosingReasonConfirmed() function used on dialog boxes which I can't find in NodeCG and isn't on the types. The dialog has had to be cast as an any before using but I am unsure of the function in general, so I left it as is.

I don't properly remember why but it appears to be related to paper-dialog-behaviour, as can be found here. I forget what it does though, so leaving the type as an any override seems OK for now until I look into it more. Edit: seems like I use it because I decided not to use NodeCG's default buttons, and needed to have 'dialog-dismissed' and 'dialog-confirmed' triggered correctly. Seems to still work fine on the v2 rewrite.

EwanLyon commented 1 year ago

Do you know why you had to set it to false? Was something failing? I should give it a try myself so will do that in a bit.

I was running into a bunch ts-loader errors related to ts-loader itself and not type issues. Setting it back to true works now again. Not sure if it was actually because the types were wrong, but I remember googling a lot trying to solve the issue.

_updateClosingReasonConfirmed()

Since it is an undocumented function being called I think it's fine to be kept as an any.

zoton2 commented 1 year ago

Everything else seems OK with this, may make some tweaks myself later but will probably merge it first. Not sure if I want to merge this into another branch on this repository, or just stick to master, as usually I just keep all development work in there, but this may need it's own branch as it's a "major" change.